[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.
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index 15bd512..1bed227 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -40,6 +40,7 @@
mlir::OperationState &result,
mlir::OpAsmParser::UnresolvedOperand &selector,
mlir::Type &type);
+bool useStrictVolatileVerification();
static constexpr llvm::StringRef getNormalizedLowerBoundAttrName() {
return "normalized.lb";
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 8a24608..05ef691 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/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 @@
// 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 @@
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>(
diff --git a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
index c5ed767..eef1377 100644
--- a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
+++ b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
@@ -423,8 +423,9 @@
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();
diff --git a/flang/test/Fir/invalid.fir b/flang/test/Fir/invalid.fir
index 447a6c6..f9f5e26 100644
--- a/flang/test/Fir/invalid.fir
+++ b/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>
diff --git a/flang/test/Fir/volatile.fir b/flang/test/Fir/volatile.fir
index 6b3d870..9a78530 100644
--- a/flang/test/Fir/volatile.fir
+++ b/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() {
diff --git a/flang/test/Fir/volatile2.fir b/flang/test/Fir/volatile2.fir
index 82a8413..d7c7351 100644
--- a/flang/test/Fir/volatile2.fir
+++ b/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
diff --git a/flang/test/HLFIR/volatile.fir b/flang/test/HLFIR/volatile.fir
index 453413a..6d43bf2 100644
--- a/flang/test/HLFIR/volatile.fir
+++ b/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
diff --git a/flang/test/HLFIR/volatile1.fir b/flang/test/HLFIR/volatile1.fir
index 174acd7..c6150fe 100644
--- a/flang/test/HLFIR/volatile1.fir
+++ b/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
diff --git a/flang/test/HLFIR/volatile2.fir b/flang/test/HLFIR/volatile2.fir
index 86ac683..0501cfc 100644
--- a/flang/test/HLFIR/volatile2.fir
+++ b/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>
diff --git a/flang/test/HLFIR/volatile3.fir b/flang/test/HLFIR/volatile3.fir
index 41e4291..24ea4e4 100644
--- a/flang/test/HLFIR/volatile3.fir
+++ b/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
diff --git a/flang/test/HLFIR/volatile4.fir b/flang/test/HLFIR/volatile4.fir
index cbf0aa3..8980bcf 100644
--- a/flang/test/HLFIR/volatile4.fir
+++ b/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
diff --git a/flang/test/Lower/volatile-allocatable1.f90 b/flang/test/Lower/volatile-allocatable1.f90
new file mode 100644
index 0000000..a21359c
--- /dev/null
+++ b/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
diff --git a/flang/test/Lower/volatile-openmp.f90 b/flang/test/Lower/volatile-openmp.f90
index 64fd5f0..28f0bf7 100644
--- a/flang/test/Lower/volatile-openmp.f90
+++ b/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
diff --git a/flang/test/Lower/volatile-string.f90 b/flang/test/Lower/volatile-string.f90
index 9173268..88b21d7 100644
--- a/flang/test/Lower/volatile-string.f90
+++ b/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
diff --git a/flang/test/Lower/volatile1.f90 b/flang/test/Lower/volatile1.f90
index 8447704..385b9fa 100644
--- a/flang/test/Lower/volatile1.f90
+++ b/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)
diff --git a/flang/test/Lower/volatile2.f90 b/flang/test/Lower/volatile2.f90
index 4b7f185..defacf8 100644
--- a/flang/test/Lower/volatile2.f90
+++ b/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()
diff --git a/flang/test/Lower/volatile3.f90 b/flang/test/Lower/volatile3.f90
index dee6642..8825f8f 100644
--- a/flang/test/Lower/volatile3.f90
+++ b/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
diff --git a/flang/test/Lower/volatile4.f90 b/flang/test/Lower/volatile4.f90
index 42d7b68..83ce2b8 100644
--- a/flang/test/Lower/volatile4.f90
+++ b/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)
