Skip to content

Commit 2f6c0e6

Browse files
authored
[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 #92036
1 parent f090801 commit 2f6c0e6

File tree

3 files changed

+43
-1
lines changed

3 files changed

+43
-1
lines changed

flang/lib/Optimizer/Analysis/AliasAnalysis.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ static bool isDummyArgument(mlir::Value v) {
3333
if (!blockArg)
3434
return false;
3535

36-
return blockArg.getOwner()->isEntryBlock();
36+
mlir::Block *owner = blockArg.getOwner();
37+
if (!owner->isEntryBlock() ||
38+
!mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp()))
39+
return false;
40+
return true;
3741
}
3842

3943
/// Temporary function to skip through all the no op operations

flang/lib/Optimizer/Transforms/AddAliasTags.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ static std::string getFuncArgName(mlir::Value arg) {
105105
"arg is a function argument");
106106
mlir::FunctionOpInterface func = mlir::dyn_cast<mlir::FunctionOpInterface>(
107107
blockArg.getOwner()->getParentOp());
108+
assert(func && "This is not a function argument");
108109
mlir::StringAttr attr = func.getArgAttrOfType<mlir::StringAttr>(
109110
blockArg.getArgNumber(), "fir.bindc_name");
110111
if (!attr)

flang/test/Transforms/tbaa.fir

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,40 @@
173173
// CHECK: fir.store %[[VAL_8]] to %[[VAL_12]] : !fir.ref<i32>
174174
// CHECK: return
175175
// CHECK: }
176+
177+
// -----
178+
179+
// Make sure we don't mistake other block arguments as dummy arguments:
180+
181+
omp.declare_reduction @add_reduction_i32 : i32 init {
182+
^bb0(%arg0: i32):
183+
%c0_i32 = arith.constant 0 : i32
184+
omp.yield(%c0_i32 : i32)
185+
} combiner {
186+
^bb0(%arg0: i32, %arg1: i32):
187+
%0 = arith.addi %arg0, %arg1 : i32
188+
omp.yield(%0 : i32)
189+
}
190+
191+
func.func @_QQmain() attributes {fir.bindc_name = "reduce"} {
192+
%c10_i32 = arith.constant 10 : i32
193+
%c6_i32 = arith.constant 6 : i32
194+
%c-1_i32 = arith.constant -1 : i32
195+
%0 = fir.address_of(@_QFEi) : !fir.ref<i32>
196+
%1 = fir.declare %0 {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
197+
omp.parallel reduction(@add_reduction_i32 %1 -> %arg0 : !fir.ref<i32>) {
198+
// CHECK: omp.parallel reduction({{.*}}) {
199+
%8 = fir.declare %arg0 {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
200+
// CHECK-NEXT: %[[DECL:.*]] = fir.declare
201+
fir.store %c-1_i32 to %8 : !fir.ref<i32>
202+
// CHECK-NOT: fir.store %{{.*}} to %[[DECL]] {tbaa = %{{.*}}} : !fir.ref<i32>
203+
// CHECK: fir.store %{{.*}} to %[[DECL]] : !fir.ref<i32>
204+
omp.terminator
205+
}
206+
return
207+
}
208+
209+
fir.global internal @_QFEi : i32 {
210+
%c0_i32 = arith.constant 0 : i32
211+
fir.has_value %c0_i32 : i32
212+
}

0 commit comments

Comments
 (0)