[flang] Hide strict volatility checks behind flag (#138183)

Enabling volatility lowering by default revealed some issues in lowering
and op verification.

For example, given volatile variable of a nested type, accessing
structure members of a structure member would result in a volatility
mismatch when the inner structure member is designated (and thus a
verification error at compile time).

In other cases, I found correct codegen when the checks were disabled,
also related to allocatable types and how we handle volatile references
of boxes.

This hides the strict verification of fir and hlfir ops behind a flag so
I can iteratively improve lowering of volatile variables without causing
compile-time failures, keeping the strict verification on when running
tests.

Author: ashermancinelli

flang/include/flang/Optimizer/Dialect/FIROps.h

@@ -40,6 +40,7 @@ mlir::ParseResult parseSelector(mlir::OpAsmParser &parser,
                                 mlir::OperationState &result,
                                 mlir::OpAsmParser::UnresolvedOperand &selector,
                                 mlir::Type &type);
+bool useStrictVolatileVerification();
 
 static constexpr llvm::StringRef getNormalizedLowerBoundAttrName() {
   return "normalized.lb";

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -33,11 +33,21 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/TypeSwitch.h"
+#include "llvm/Support/CommandLine.h"
 
 namespace {
 #include "flang/Optimizer/Dialect/CanonicalizationPatterns.inc"
 } // namespace
 
+static llvm::cl::opt<bool> clUseStrictVolatileVerification(
+    "strict-fir-volatile-verifier", llvm::cl::init(false),
+    llvm::cl::desc(
+        "use stricter verifier for FIR operations with volatile types"));
+
+bool fir::useStrictVolatileVerification() {
+  return clUseStrictVolatileVerification;
+}
+
 static void propagateAttributes(mlir::Operation *fromOp,
                                 mlir::Operation *toOp) {
   if (!fromOp || !toOp)
@@ -1535,11 +1545,14 @@ llvm::LogicalResult fir::ConvertOp::verify() {
   // represent volatility.
   const bool toLLVMPointer = mlir::isa<mlir::LLVM::LLVMPointerType>(outType);
   const bool toInteger = fir::isa_integer(outType);
-  if (fir::isa_volatile_type(inType) != fir::isa_volatile_type(outType) &&
-      !toLLVMPointer && !toInteger)
-    return emitOpError("cannot convert between volatile and non-volatile "
-                       "types, use fir.volatile_cast instead ")
-           << inType << " / " << outType;
+  if (fir::useStrictVolatileVerification()) {
+    if (fir::isa_volatile_type(inType) != fir::isa_volatile_type(outType) &&
+        !toLLVMPointer && !toInteger) {
+      return emitOpError("cannot convert between volatile and non-volatile "
+                         "types, use fir.volatile_cast instead ")
+             << inType << " / " << outType;
+    }
+  }
   if (canBeConverted(inType, outType))
     return mlir::success();
   return emitOpError("invalid type conversion")
@@ -1841,6 +1854,10 @@ llvm::LogicalResult fir::TypeInfoOp::verify() {
 static llvm::LogicalResult
 verifyEmboxOpVolatilityInvariants(mlir::Type memrefType,
                                   mlir::Type resultType) {
+
+  if (!fir::useStrictVolatileVerification())
+    return mlir::success();
+
   mlir::Type boxElementType =
       llvm::TypeSwitch<mlir::Type, mlir::Type>(resultType)
           .Case<fir::BoxType, fir::ClassType>(

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

@@ -423,8 +423,9 @@ llvm::LogicalResult hlfir::DesignateOp::verify() {
   unsigned outputRank = 0;
   mlir::Type outputElementType;
   bool hasBoxComponent;
-  if (fir::isa_volatile_type(memrefType) !=
-      fir::isa_volatile_type(getResult().getType())) {
+  if (fir::useStrictVolatileVerification() &&
+      fir::isa_volatile_type(memrefType) !=
+          fir::isa_volatile_type(getResult().getType())) {
     return emitOpError("volatility mismatch between memref and result type")
            << " memref type: " << memrefType
            << " result type: " << getResult().getType();

flang/test/Fir/invalid.fir

@@ -1,6 +1,6 @@
-// FIR ops diagnotic tests
 
-// RUN: fir-opt -split-input-file -verify-diagnostics %s
+
+// RUN: fir-opt -split-input-file -verify-diagnostics --strict-fir-volatile-verifier %s
 
 // expected-error@+1{{custom op 'fir.string_lit' must have character type}}
 %0 = fir.string_lit "Hello, World!"(13) : !fir.int<32>

flang/test/Fir/volatile.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s -o - | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s -o - | FileCheck %s
 // CHECK: llvm.store volatile %{{.+}}, %{{.+}} : i32, !llvm.ptr
 // CHECK: %{{.+}} = llvm.load volatile %{{.+}} : !llvm.ptr -> i32
 func.func @foo() {

flang/test/Fir/volatile2.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt --fir-to-llvm-ir %s | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier --fir-to-llvm-ir %s | FileCheck %s
 func.func @_QQmain() {
   %0 = fir.alloca !fir.box<!fir.array<10xi32>, volatile>
   %c1 = arith.constant 1 : index

flang/test/HLFIR/volatile.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt --convert-hlfir-to-fir %s -o - | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier --convert-hlfir-to-fir %s -o - | FileCheck %s
 
 func.func @foo() {
   %true = arith.constant true

flang/test/HLFIR/volatile1.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
 func.func @_QQmain() attributes {fir.bindc_name = "p"} {
   %0 = fir.address_of(@_QFEarr) : !fir.ref<!fir.array<10xi32>>
   %c10 = arith.constant 10 : index

flang/test/HLFIR/volatile2.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
 func.func private @_QFPa() -> i32 attributes {fir.host_symbol = @_QQmain, llvm.linkage = #llvm.linkage<internal>} {
   %0 = fir.alloca i32 {bindc_name = "a", uniq_name = "_QFFaEa"}
   %1 = fir.volatile_cast %0 : (!fir.ref<i32>) -> !fir.ref<i32, volatile>

flang/test/HLFIR/volatile3.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
 func.func @_QQmain() attributes {fir.bindc_name = "p"} {
   %0 = fir.address_of(@_QFEarr) : !fir.ref<!fir.array<10xi32>>
   %c10 = arith.constant 10 : index

flang/test/HLFIR/volatile4.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
+// RUN: fir-opt --strict-fir-volatile-verifier %s --bufferize-hlfir --convert-hlfir-to-fir | FileCheck %s
 func.func @_QQmain() attributes {fir.bindc_name = "p"} {
   %0 = fir.address_of(@_QFEarr) : !fir.ref<!fir.array<10xi32>>
   %c10 = arith.constant 10 : index

flang/test/Lower/volatile-allocatable1.f90

@@ -0,0 +1,17 @@
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
+
+! Requires correct propagation of volatility for allocatable nested types.
+! XFAIL: *
+
+function allocatable_udt()
+  type :: base_type
+    integer :: i = 42
+  end type
+  type, extends(base_type) :: ext_type
+    integer :: j = 100
+  end type
+  integer :: allocatable_udt
+  type(ext_type), allocatable, volatile :: v2(:,:)
+  allocate(v2(2,3))
+  allocatable_udt = v2(1,1)%i
+end function

flang/test/Lower/volatile-openmp.f90

@@ -1,4 +1,4 @@
-! RUN: bbc -fopenmp %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier -fopenmp %s -o - | FileCheck %s
 type t
     integer, pointer :: array(:)
 end type

flang/test/Lower/volatile-string.f90

@@ -1,4 +1,4 @@
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
 program p
   character(3), volatile :: string = 'foo'
   character(3)           :: nonvolatile_string

flang/test/Lower/volatile1.f90

@@ -1,4 +1,4 @@
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
 
 program p
   integer,volatile::i,arr(10)

flang/test/Lower/volatile2.f90

@@ -1,4 +1,4 @@
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
 
 program p
   print*,a(),b(),c()

flang/test/Lower/volatile3.f90

@@ -1,4 +1,4 @@
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
 
 ! Test that all combinations of volatile pointer and target are properly lowered -
 ! note that a volatile pointer implies that the target is volatile, even if not specified

flang/test/Lower/volatile4.f90

@@ -1,4 +1,4 @@
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
 
 program p
   integer,volatile::i,arr(10)