llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Steffen Larsen (steffenlarsen) <details> <summary>Changes</summary> HoistAllocas returns early when an alloca was already in the entry block, but being in the block is not the same as being ahead of the control flow in it. The pass runs before FlattenCFG, which splits the body at each structured control-flow op, so an alloca sitting after one is left in a block that is no longer the entry block. That matters because SROA only collects allocas from the entry block. An alloca stranded behind a scope is invisible to it and never promoted, so anything whose address it holds stays in memory. --- Full diff: https://github.com/llvm/llvm-project/pull/220496.diff 2 Files Affected: - (modified) clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp (+14-2) - (modified) clang/test/CIR/Transforms/hoist-allocas.cir (+27) ``````````diff diff --git a/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp index 5c41541ec76bf..1eb3e2524e124 100644 --- a/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp +++ b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp @@ -64,11 +64,23 @@ static void process(mlir::ModuleOp mod, cir::FuncOp func) { // let's not depend on the default staying that way. func.getBody().walk<mlir::WalkOrder::PostOrder>([&](cir::AllocaOp alloca) { mlir::Block *destBlock = getHoistDestBlock(alloca); - if (alloca->getBlock() == destBlock) - return; // Don't hoist allocas with dynamic alloca size. if (alloca.getDynAllocSize()) return; + if (alloca->getBlock() == destBlock) { + // Already in the right block, but not necessarily ahead of the control + // flow in it. This pass runs before FlattenCFG, which splits the body at + // each structured control-flow op, so an alloca that sits after one ends + // up in a non-entry block and SROA only ever collects allocas from the + // entry block, so it would never be promoted at all. + // Only a region-carrying op ahead of the alloca can strand it that way; + // straight-line code cannot. + if (std::all_of(destBlock->begin(), alloca->getIterator(), + [](mlir::Operation &blockOp) { + return blockOp.getNumRegions() == 0; + })) + return; + } // Hoist allocas into the entry block. diff --git a/clang/test/CIR/Transforms/hoist-allocas.cir b/clang/test/CIR/Transforms/hoist-allocas.cir index db04030412252..222be921b3a17 100644 --- a/clang/test/CIR/Transforms/hoist-allocas.cir +++ b/clang/test/CIR/Transforms/hoist-allocas.cir @@ -196,3 +196,30 @@ module { // CHECK-NEXT: cir.return // CHECK-NEXT: } } + +cir.func @alloca_after_scope() { + cir.scope { + %0 = cir.alloca "inner" align(4) init : !cir.ptr<!s32i> + } + %1 = cir.alloca "after" align(4) init : !cir.ptr<!s32i> + cir.return +} + +// CHECK: cir.func{{.*}} @alloca_after_scope() +// CHECK-NEXT: cir.alloca{{.*}}"inner" +// CHECK-NEXT: cir.alloca{{.*}}"after" +// CHECK-NEXT: cir.scope + +cir.func @alloca_after_straightline() { + %0 = cir.alloca "first" align(4) init : !cir.ptr<!s32i> + %1 = cir.const #cir.int<0> : !s32i + cir.store %1, %0 : !s32i, !cir.ptr<!s32i> + %2 = cir.alloca "second" align(4) init : !cir.ptr<!s32i> + cir.return +} + +// CHECK: cir.func{{.*}} @alloca_after_straightline() +// CHECK-NEXT: cir.alloca{{.*}}"first" +// CHECK-NEXT: cir.const +// CHECK-NEXT: cir.store +// CHECK-NEXT: cir.alloca{{.*}}"second" `````````` </details> https://github.com/llvm/llvm-project/pull/220496 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
