Skip to content

Commit b28f5e1

Browse files
committed
[flang][CodeGen] fix bug hoisting allocas using a shared constant arg
When hoisting the allocas with a constant integer size, the constant integer was moved to where the alloca is hoisted to unconditionally. By CodeGen there have been various iterations of mlir canonicalization and dead code elimination. This can cause lots of unrelated bits of code to share the same constant values. If for some reason the alloca couldn't be hoisted all of the way to the entry block of the function, moving the constant might result in it no-longer dominating some of the remaining uses. In theory, there should be dominance analysis to ensure the location of the constant does dominate all uses of it. But those constants are effectively free anyway (they aren't even separate instructions in LLVM IR), so it is less expensive just to leave the old one where it was and insert a new one we know for sure is immediately before the alloca.
1 parent e5d5ee4 commit b28f5e1

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

flang/lib/Optimizer/CodeGen/CodeGen.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,14 @@ struct AllocaOpConversion : public fir::FIROpConversion<fir::AllocaOp> {
278278
mlir::Region *parentRegion = rewriter.getInsertionBlock()->getParent();
279279
mlir::Block *insertBlock =
280280
getBlockForAllocaInsert(parentOp, parentRegion);
281-
size.getDefiningOp()->moveBefore(&insertBlock->front());
281+
282+
// The old size might have had multiple users, some at a broader scope
283+
// than we can safely outline the alloca to. As it is only an
284+
// llvm.constant operation, it is faster to clone it than to calculate the
285+
// dominance to see if it really should be moved.
286+
mlir::Operation *clonedSize = rewriter.clone(*size.getDefiningOp());
287+
size = clonedSize->getResult(0);
288+
clonedSize->moveBefore(&insertBlock->front());
282289
rewriter.setInsertionPointAfter(size.getDefiningOp());
283290
}
284291

flang/test/Fir/convert-to-llvm-openmp-and-fir.fir

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,3 +1065,51 @@ func.func @omp_map_common_block_using_common_block_members() {
10651065
}
10661066

10671067
fir.global common @var_common_(dense<0> : vector<8xi8>) {alignment = 4 : i64} : !fir.array<8xi8>
1068+
1069+
// -----
1070+
1071+
1072+
func.func @use_string(%arg0 : !fir.ref<!fir.char<1,?>>) {
1073+
return
1074+
}
1075+
1076+
func.func @use_index(%arg0 : index) {
1077+
return
1078+
}
1079+
1080+
// CHECK-LABEL: llvm.func @alloca_hoisting_openmp() {
1081+
// CHECK: %[[VAL_0:.*]] = llvm.mlir.constant(6 : index) : i64
1082+
// CHECK: %[[VAL_1:.*]] = llvm.mlir.constant(1 : i32) : i32
1083+
// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(42 : i32) : i32
1084+
// CHECK: omp.parallel {
1085+
// CHECK: %[[VAL_3:.*]] = llvm.mlir.constant(6 : index) : i64
1086+
// CHECK: %[[VAL_4:.*]] = llvm.alloca %[[VAL_3]] x i8 : (i64) -> !llvm.ptr
1087+
// CHECK: omp.wsloop {
1088+
// CHECK: omp.loop_nest (%[[VAL_5:.*]]) : i32 = (%[[VAL_1]]) to (%[[VAL_2]]) inclusive step (%[[VAL_1]]) {
1089+
// CHECK: %[[VAL_6:.*]] = llvm.mlir.constant(1 : i64) : i64
1090+
// CHECK: llvm.call @use_string(%[[VAL_4]]) : (!llvm.ptr) -> ()
1091+
// CHECK: omp.yield
1092+
// CHECK: }
1093+
// CHECK: }
1094+
// CHECK: omp.terminator
1095+
// CHECK: }
1096+
// CHECK: llvm.call @use_index(%[[VAL_0]]) : (i64) -> ()
1097+
// CHECK: llvm.return
1098+
// CHECK: }
1099+
func.func @alloca_hoisting_openmp() {
1100+
%c6 = arith.constant 6 : index
1101+
%c1_i32 = arith.constant 1 : i32
1102+
%c42_i32 = arith.constant 42 : i32
1103+
omp.parallel {
1104+
omp.wsloop {
1105+
omp.loop_nest (%arg0) : i32 = (%c1_i32) to (%c42_i32) inclusive step (%c1_i32) {
1106+
%0 = fir.alloca !fir.char<1,?>(%c6 : index)
1107+
fir.call @use_string(%0) : (!fir.ref<!fir.char<1,?>>) -> ()
1108+
omp.yield
1109+
}
1110+
}
1111+
omp.terminator
1112+
}
1113+
fir.call @use_index(%c6) : (index) -> ()
1114+
return
1115+
}

0 commit comments

Comments
 (0)