Skip to content

Commit 90da616

Browse files
vzakharivzakhari
authored
[flang] Use outermost fir.dummy_scope for TBAA of local allocations. (#146006)
This change only matters when MLIR function inlining kicks in, and I am still just experimenting with this. The added LIT test shows the particular example where the current TBAA assignment is incorrect: ``` subroutine bar(this) ... end subroutine bar function foo() result(res) type(t) :: res call bar(res) end function foo subroutine test(arg) ! %0 = fir.alloca !fir.type<_QMmTt{x:f32}> {bindc_name = ".result"} type(t) :: var var = foo() ! fir.save_result foo's result to %0 (temp-alloca) ! fir.declare %0 {uniq_name = ".tmp.func_result"} ! load from %0 -> store into var arg = var%x end subroutine test ``` One way for FIR inlining to handle `foo()` call is to substitute all uses of the `res` alloca (inside `foo`) with uses of the temp-alloca (inside `test`). This means that the temp-alloca is then used by the code inside bar (e.g. to store something into it). After the inlining, the innermost dominating fir.dummy_scope for `fir.declare %0` is the one from `bar`. If we use this scope for assigning TBAA to the load from temp-alloca, then it will be in the same TBAA tree as the stores into `this` inside `bar`. The load and the stores will have different sub-trees, because `this` is a dummy argument, and temp-alloca is a local allocation. So they will appear as non-conflicting while they are accessing the same memory. This change makes sure that all local allocations always use the outermost scope.
1 parent 0f291e5 commit 90da616

File tree

3 files changed

+122
-9
lines changed

3 files changed

+122
-9
lines changed

‎flang/lib/Optimizer/Transforms/AddAliasTags.cpp

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ class PassState {
7373
}
7474

7575
void processFunctionScopes(mlir::func::FuncOp func);
76-
fir::DummyScopeOp getDeclarationScope(fir::DeclareOp declareOp);
76+
// For the given fir.declare returns the dominating fir.dummy_scope
77+
// operation.
78+
fir::DummyScopeOp getDeclarationScope(fir::DeclareOp declareOp) const;
79+
// For the given fir.declare returns the outermost fir.dummy_scope
80+
// in the current function.
81+
fir::DummyScopeOp getOutermostScope(fir::DeclareOp declareOp) const;
7782

7883
private:
7984
mlir::DominanceInfo &domInfo;
@@ -119,9 +124,8 @@ void PassState::processFunctionScopes(mlir::func::FuncOp func) {
119124
}
120125
}
121126

122-
// For the given fir.declare returns the dominating fir.dummy_scope
123-
// operation.
124-
fir::DummyScopeOp PassState::getDeclarationScope(fir::DeclareOp declareOp) {
127+
fir::DummyScopeOp
128+
PassState::getDeclarationScope(fir::DeclareOp declareOp) const {
125129
auto func = declareOp->getParentOfType<mlir::func::FuncOp>();
126130
assert(func && "fir.declare does not have parent func.func");
127131
auto &scopeOps = sortedScopeOperations.at(func);
@@ -132,6 +136,15 @@ fir::DummyScopeOp PassState::getDeclarationScope(fir::DeclareOp declareOp) {
132136
return nullptr;
133137
}
134138

139+
fir::DummyScopeOp PassState::getOutermostScope(fir::DeclareOp declareOp) const {
140+
auto func = declareOp->getParentOfType<mlir::func::FuncOp>();
141+
assert(func && "fir.declare does not have parent func.func");
142+
auto &scopeOps = sortedScopeOperations.at(func);
143+
if (!scopeOps.empty())
144+
return scopeOps[0];
145+
return nullptr;
146+
}
147+
135148
class AddAliasTagsPass : public fir::impl::AddAliasTagsBase<AddAliasTagsPass> {
136149
public:
137150
void runOnOperation() override;
@@ -279,6 +292,15 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
279292
else
280293
unknownAllocOp = true;
281294

295+
if (auto declOp = source.origin.instantiationPoint) {
296+
// Use the outermost scope for local allocations,
297+
// because using the innermost scope may result
298+
// in incorrect TBAA, when calls are inlined in MLIR.
299+
auto declareOp = mlir::dyn_cast<fir::DeclareOp>(declOp);
300+
assert(declareOp && "Instantiation point must be fir.declare");
301+
scopeOp = state.getOutermostScope(declareOp);
302+
}
303+
282304
if (unknownAllocOp) {
283305
LLVM_DEBUG(llvm::dbgs().indent(2)
284306
<< "WARN: unknown defining op for SourceKind::Allocate " << *op

‎flang/test/Transforms/tbaa-for-local-vars.fir

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Test TBAA tags assignment for access off the fir.declare
2+
// placed in the middle of the routine (e.g. a temporary).
3+
// Original example:
4+
// module m
5+
// type t
6+
// real :: x
7+
// end type t
8+
// contains
9+
// subroutine bar(this)
10+
// class(t), intent(out) :: this
11+
// this%x = 1.0
12+
// end subroutine bar
13+
// function foo() result(res)
14+
// type(t) :: res
15+
// call bar(res)
16+
// end function foo
17+
// subroutine test(arg)
18+
// type(t) :: var
19+
// var = foo()
20+
// arg = var%x
21+
// end subroutine test
22+
// end module m
23+
//
24+
// The calls were manually inlined in FIR with fir.save_result's
25+
// destination operand being used instead of the temporary
26+
// alloca of the result inside foo. Runtime calls were removed
27+
// to reduce the test.
28+
// The temporary function result fir.declare is then dominated
29+
// by fir.dummy_scope of the foo function. If we use this scope
30+
// to assign the TBAA local-alloc tag to the accesses of
31+
// the temporary, then it won't conflict with the TBAA dummy tag
32+
// assigned to the accesses of this argument of bar.
33+
// That would be incorrect, because they access the same memory.
34+
// Check that the local-alloc tag is placed into the outermost
35+
// scope's TBAA tree.
36+
// RUN: fir-opt --fir-add-alias-tags %s | FileCheck %s
37+
38+
// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root<id = "Flang function root _QMmPtest - Scope 2">
39+
// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_root<id = "Flang function root _QMmPtest">
40+
// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_0]], 0>}>
41+
// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_1]], 0>}>
42+
// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_2]], 0>}>
43+
// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_3]], 0>}>
44+
// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#[[$ATTR_4]], 0>}>
45+
// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_type_desc<id = "target data", members = {<#[[$ATTR_5]], 0>}>
46+
// CHECK: #[[$ATTR_9:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data/_QMmFbarEthis", members = {<#[[$ATTR_6]], 0>}>
47+
// CHECK: #[[$ATTR_10:.+]] = #llvm.tbaa_type_desc<id = "allocated data", members = {<#[[$ATTR_7]], 0>}>
48+
// CHECK: #[[$ATTR_12:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_9]], access_type = #[[$ATTR_9]], offset = 0>
49+
// CHECK: #[[$ATTR_13:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_10]], access_type = #[[$ATTR_10]], offset = 0>
50+
51+
// CHECK-LABEL: func.func @_QMmPtest(
52+
// CHECK-SAME: %[[ARG0:.*]]: !fir.ref<f32> {fir.bindc_name = "arg"}) {
53+
// CHECK: %[[VAL_0:.*]] = arith.constant 1.000000e+00 : f32
54+
// CHECK: %[[VAL_1:.*]] = fir.alloca !fir.type<_QMmTt{x:f32}> {bindc_name = ".result"}
55+
// CHECK: %[[VAL_2:.*]] = fir.dummy_scope : !fir.dscope
56+
// CHECK: %[[VAL_3:.*]] = fir.declare %[[ARG0]] dummy_scope %[[VAL_2]] {uniq_name = "_QMmFtestEarg"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
57+
// CHECK: %[[VAL_6:.*]] = fir.dummy_scope : !fir.dscope
58+
// CHECK: %[[VAL_7:.*]] = fir.declare %[[VAL_1]] {uniq_name = "_QMmFfooEres"} : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<!fir.type<_QMmTt{x:f32}>>
59+
// CHECK: %[[VAL_8:.*]] = fir.embox %[[VAL_7]] : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.box<!fir.type<_QMmTt{x:f32}>>
60+
// CHECK: %[[VAL_9:.*]] = fir.convert %[[VAL_8]] : (!fir.box<!fir.type<_QMmTt{x:f32}>>) -> !fir.class<!fir.type<_QMmTt{x:f32}>>
61+
// CHECK: %[[VAL_10:.*]] = fir.dummy_scope : !fir.dscope
62+
// CHECK: %[[VAL_11:.*]] = fir.declare %[[VAL_9]] dummy_scope %[[VAL_10]] {fortran_attrs = #fir.var_attrs<intent_out>, uniq_name = "_QMmFbarEthis"} : (!fir.class<!fir.type<_QMmTt{x:f32}>>, !fir.dscope) -> !fir.class<!fir.type<_QMmTt{x:f32}>>
63+
// CHECK: %[[VAL_12:.*]] = fir.coordinate_of %[[VAL_11]], x : (!fir.class<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<f32>
64+
// CHECK: fir.store %[[VAL_0]] to %[[VAL_12]] {tbaa = [#[[$ATTR_12]]]} : !fir.ref<f32>
65+
// CHECK: %[[VAL_13:.*]] = fir.declare %[[VAL_1]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<!fir.type<_QMmTt{x:f32}>>
66+
// CHECK: %[[VAL_14:.*]] = fir.coordinate_of %[[VAL_13]], x : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<f32>
67+
// CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_14]] {tbaa = [#[[$ATTR_13]]]} : !fir.ref<f32>
68+
func.func @_QMmPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "arg"}) {
69+
%cst = arith.constant 1.000000e+00 : f32
70+
%0 = fir.alloca !fir.type<_QMmTt{x:f32}> {bindc_name = ".result"}
71+
%1 = fir.dummy_scope : !fir.dscope
72+
%2 = fir.declare %arg0 dummy_scope %1 {uniq_name = "_QMmFtestEarg"} : (!fir.ref<f32>, !fir.dscope) -> !fir.ref<f32>
73+
%3 = fir.alloca !fir.type<_QMmTt{x:f32}> {bindc_name = "var", uniq_name = "_QMmFtestEvar"}
74+
%4 = fir.declare %3 {uniq_name = "_QMmFtestEvar"} : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<!fir.type<_QMmTt{x:f32}>>
75+
%5 = fir.dummy_scope : !fir.dscope
76+
%6 = fir.declare %0 {uniq_name = "_QMmFfooEres"} : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<!fir.type<_QMmTt{x:f32}>>
77+
%7 = fir.embox %6 : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.box<!fir.type<_QMmTt{x:f32}>>
78+
%8 = fir.convert %7 : (!fir.box<!fir.type<_QMmTt{x:f32}>>) -> !fir.class<!fir.type<_QMmTt{x:f32}>>
79+
%9 = fir.dummy_scope : !fir.dscope
80+
%10 = fir.declare %8 dummy_scope %9 {fortran_attrs = #fir.var_attrs<intent_out>, uniq_name = "_QMmFbarEthis"} : (!fir.class<!fir.type<_QMmTt{x:f32}>>, !fir.dscope) -> !fir.class<!fir.type<_QMmTt{x:f32}>>
81+
%14 = fir.coordinate_of %10, x : (!fir.class<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<f32>
82+
fir.store %cst to %14 : !fir.ref<f32>
83+
%15 = fir.declare %0 {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<!fir.type<_QMmTt{x:f32}>>
84+
%16 = fir.coordinate_of %15, x : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<f32>
85+
%17 = fir.coordinate_of %4, x : (!fir.ref<!fir.type<_QMmTt{x:f32}>>) -> !fir.ref<f32>
86+
%18 = fir.load %16 : !fir.ref<f32>
87+
fir.store %18 to %17 : !fir.ref<f32>
88+
%19 = fir.load %17 : !fir.ref<f32>
89+
fir.store %19 to %2 : !fir.ref<f32>
90+
return
91+
}
92+

‎flang/test/Transforms/tbaa-with-dummy-scope2.fir

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,17 @@ func.func @_QPtest2() attributes {noinline} {
8484
fir.store %c2_i32 to %2 : !fir.ref<i32>
8585
return
8686
}
87-
// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2 - Scope 1">
88-
// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2">
87+
// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2">
88+
// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2 - Scope 1">
8989
// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_0]], 0>}>
9090
// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_1]], 0>}>
9191
// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_2]], 0>}>
9292
// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_3]], 0>}>
9393
// CHECK: #[[$TARGETDATA_0:.+]] = #llvm.tbaa_type_desc<id = "target data", members = {<#[[$ATTR_4]], 0>}>
94-
// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#[[$ATTR_4]], 0>}>
95-
// CHECK: #[[$TARGETDATA_1:.+]] = #llvm.tbaa_type_desc<id = "target data", members = {<#[[$ATTR_5]], 0>}>
94+
// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#[[$ATTR_5]], 0>}>
9695
// CHECK: #[[$LOCAL_ATTR_0:.+]] = #llvm.tbaa_type_desc<id = "allocated data", members = {<#[[$TARGETDATA_0]], 0>}>
9796
// CHECK: #[[$ATTR_8:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data/_QFtest2FinnerEx", members = {<#[[$ATTR_6]], 0>}>
98-
// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_type_desc<id = "global data", members = {<#[[$TARGETDATA_1]], 0>}>
97+
// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_type_desc<id = "global data", members = {<#[[$TARGETDATA_0]], 0>}>
9998
// CHECK: #[[$ATTR_10:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_8]], access_type = #[[$ATTR_8]], offset = 0>
10099
// CHECK: #[[$LOCAL_ATTR_1:.+]] = #llvm.tbaa_type_desc<id = "allocated data/_QFtest2FinnerEy", members = {<#[[$LOCAL_ATTR_0]], 0>}>
101100
// CHECK: #[[$ATTR_9:.+]] = #llvm.tbaa_type_desc<id = "global data/_QMmEglob", members = {<#[[$ATTR_7]], 0>}>

0 commit comments

Comments
 (0)