[flang-commits] [flang] 2f6c0e6 - [flang][Alias Analysis] not all block arguments are dummy arguments (#92156)

[via flang-commits]

Author: Tom Eccles
Date: 2024-05-15T10:25:51+01:00
New Revision: 2f6c0e6e180c81087c26f4afac2155ea70472ec6

URL: https://github.com/llvm/llvm-project/commit/2f6c0e6e180c81087c26f4afac2155ea70472ec6
DIFF: https://github.com/llvm/llvm-project/commit/2f6c0e6e180c81087c26f4afac2155ea70472ec6.diff

LOG: [flang][Alias Analysis] not all block arguments are dummy arguments (#92156)

Arguments to openmp regions should not be tagged as dummy arguments.
This is particularly unsafe because these openmp blocks will eventually
be inlined into the calling function, where they will trivially alias
with other values inside of the calling function.

This is probably a theoretical issue because the calls to openmp runtime
function calls would act as barriers, preventing optimizations that are
too aggressive. But a lot more thought would need to go into a bet like
that.

This came out of discussion on
https://github.com/llvm/llvm-project/pull/92036

Added: 
    

Modified: 
    flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
    flang/lib/Optimizer/Transforms/AddAliasTags.cpp
    flang/test/Transforms/tbaa.fir

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index f723e8f66e3e4..ed1101dc5e8d8 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -33,7 +33,11 @@ static bool isDummyArgument(mlir::Value v) {
   if (!blockArg)
     return false;
 
-  return blockArg.getOwner()->isEntryBlock();
+  mlir::Block *owner = blockArg.getOwner();
+  if (!owner->isEntryBlock() ||
+      !mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp()))
+    return false;
+  return true;
 }
 
 /// Temporary function to skip through all the no op operations

diff  --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 684aa4462915e..3642a812096db 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -105,6 +105,7 @@ static std::string getFuncArgName(mlir::Value arg) {
          "arg is a function argument");
   mlir::FunctionOpInterface func = mlir::dyn_cast<mlir::FunctionOpInterface>(
       blockArg.getOwner()->getParentOp());
+  assert(func && "This is not a function argument");
   mlir::StringAttr attr = func.getArgAttrOfType<mlir::StringAttr>(
       blockArg.getArgNumber(), "fir.bindc_name");
   if (!attr)

diff  --git a/flang/test/Transforms/tbaa.fir b/flang/test/Transforms/tbaa.fir
index 7825ae60c71e6..f94bbe4bf9485 100644
--- a/flang/test/Transforms/tbaa.fir
+++ b/flang/test/Transforms/tbaa.fir
@@ -173,3 +173,40 @@
 // CHECK:           fir.store %[[VAL_8]] to %[[VAL_12]] : !fir.ref<i32>
 // CHECK:           return
 // CHECK:         }
+
+// -----
+
+// Make sure we don't mistake other block arguments as dummy arguments:
+
+omp.declare_reduction @add_reduction_i32 : i32 init {
+^bb0(%arg0: i32):
+  %c0_i32 = arith.constant 0 : i32
+  omp.yield(%c0_i32 : i32)
+} combiner {
+^bb0(%arg0: i32, %arg1: i32):
+  %0 = arith.addi %arg0, %arg1 : i32
+  omp.yield(%0 : i32)
+}
+
+func.func @_QQmain() attributes {fir.bindc_name = "reduce"} {
+  %c10_i32 = arith.constant 10 : i32
+  %c6_i32 = arith.constant 6 : i32
+  %c-1_i32 = arith.constant -1 : i32
+  %0 = fir.address_of(@_QFEi) : !fir.ref<i32>
+  %1 = fir.declare %0 {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
+  omp.parallel reduction(@add_reduction_i32 %1 -> %arg0 : !fir.ref<i32>) {
+// CHECK: omp.parallel reduction({{.*}}) {
+    %8 = fir.declare %arg0 {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
+// CHECK-NEXT: %[[DECL:.*]] = fir.declare
+    fir.store %c-1_i32 to %8 : !fir.ref<i32>
+// CHECK-NOT: fir.store %{{.*}} to %[[DECL]] {tbaa = %{{.*}}} : !fir.ref<i32>
+// CHECK:     fir.store %{{.*}} to %[[DECL]] : !fir.ref<i32>
+    omp.terminator
+  }
+  return
+}
+
+fir.global internal @_QFEi : i32 {
+  %c0_i32 = arith.constant 0 : i32
+  fir.has_value %c0_i32 : i32
+}