[CIR] Emit allocas into the proper lexical scope #132468
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andykaylor
requested review from
AmrDeveloper,
bcardosolopes,
dkolsen-pgi,
erichkeane and
lanza
llvmbot
added
clang
Contributor
Author
|
CC: @mmha |
Member
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesAlloca operations were being emitted into the entry block of the current Full diff: https://github.com/llvm/llvm-project/pull/132468.diff 13 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Passes.h b/clang/include/clang/CIR/Dialect/Passes.h
index aa84241bdecf0..133eb462dcf1f 100644
--- a/clang/include/clang/CIR/Dialect/Passes.h
+++ b/clang/include/clang/CIR/Dialect/Passes.h
@@ -22,6 +22,7 @@ namespace mlir {
std::unique_ptr<Pass> createCIRCanonicalizePass();
std::unique_ptr<Pass> createCIRFlattenCFGPass();
+std::unique_ptr<Pass> createHoistAllocasPass();
void populateCIRPreLoweringPasses(mlir::OpPassManager &pm);
diff --git a/clang/include/clang/CIR/Dialect/Passes.td b/clang/include/clang/CIR/Dialect/Passes.td
index 16133d020a7c8..74c255861c879 100644
--- a/clang/include/clang/CIR/Dialect/Passes.td
+++ b/clang/include/clang/CIR/Dialect/Passes.td
@@ -29,6 +29,16 @@ def CIRCanonicalize : Pass<"cir-canonicalize"> {
let dependentDialects = ["cir::CIRDialect"];
}
+def HoistAllocas : Pass<"cir-hoist-allocas"> {
+ let summary = "Hoist allocas to the entry of the function";
+ let description = [{
+ This pass hoist all non-dynamic allocas to the entry of the function.
+ This is helpful for later code generation.
+ }];
+ let constructor = "mlir::createHoistAllocasPass()";
+ let dependentDialects = ["cir::CIRDialect"];
+}
+
def CIRFlattenCFG : Pass<"cir-flatten-cfg"> {
let summary = "Produces flatten CFG";
let description = [{
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 3e33e5dc60194..3f2e56a43fbef 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -127,6 +127,9 @@ struct MissingFeatures {
static bool ternaryOp() { return false; }
static bool tryOp() { return false; }
static bool zextOp() { return false; }
+
+ // Future CIR attributes
+ static bool optInfoAttr() { return false; }
};
} // namespace cir
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index a93e8dbcb42de..f2153c23ebb43 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -49,7 +49,8 @@ CIRGenFunction::emitAutoVarAlloca(const VarDecl &d) {
// A normal fixed sized variable becomes an alloca in the entry block,
mlir::Type allocaTy = convertTypeForMem(ty);
// Create the temp alloca and declare variable using it.
- address = createTempAlloca(allocaTy, alignment, loc, d.getName());
+ address = createTempAlloca(allocaTy, alignment, loc, d.getName(),
+ /*insertIntoFnEntryBlock=*/false);
declare(address.getPointer(), &d, ty, getLoc(d.getSourceRange()), alignment);
emission.Addr = address;
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 306130b80d457..d16480143c47e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -261,10 +261,27 @@ void CIRGenFunction::emitIgnoredExpr(const Expr *e) {
}
mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
- mlir::Location loc,
- CharUnits alignment) {
- mlir::Block *entryBlock = getCurFunctionEntryBlock();
+ mlir::Location loc, CharUnits alignment,
+ bool insertIntoFnEntryBlock,
+ mlir::Value arraySize) {
+ mlir::Block *entryBlock = insertIntoFnEntryBlock
+ ? getCurFunctionEntryBlock()
+ : curLexScope->getEntryBlock();
+
+ // If this is an alloca in the entry basic block of a cir.try and there's
+ // a surrounding cir.scope, make sure the alloca ends up in the surrounding
+ // scope instead. This is necessary in order to guarantee all SSA values are
+ // reachable during cleanups.
+ assert(!cir::MissingFeatures::tryOp());
+
+ return emitAlloca(name, ty, loc, alignment,
+ builder.getBestAllocaInsertPoint(entryBlock), arraySize);
+}
+mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
+ mlir::Location loc, CharUnits alignment,
+ mlir::OpBuilder::InsertPoint ip,
+ mlir::Value arraySize) {
// CIR uses its own alloca address space rather than follow the target data
// layout like original CodeGen. The data layout awareness should be done in
// the lowering pass instead.
@@ -275,7 +292,7 @@ mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
mlir::Value addr;
{
mlir::OpBuilder::InsertionGuard guard(builder);
- builder.restoreInsertionPoint(builder.getBestAllocaInsertPoint(entryBlock));
+ builder.restoreInsertionPoint(ip);
addr = builder.createAlloca(loc, /*addr type*/ localVarPtrTy,
/*var type*/ ty, name, alignIntAttr);
assert(!cir::MissingFeatures::astVarDeclInterface());
@@ -290,11 +307,13 @@ mlir::Value CIRGenFunction::createDummyValue(mlir::Location loc,
return builder.createDummyValue(loc, t, alignment);
}
-/// This creates an alloca and inserts it at the current insertion point of the
-/// builder.
+/// This creates an alloca and inserts it into the entry block if
+/// \p insertIntoFnEntryBlock is true, otherwise it inserts it at the current
+/// insertion point of the builder.
Address CIRGenFunction::createTempAlloca(mlir::Type ty, CharUnits align,
- mlir::Location loc,
- const Twine &name) {
- mlir::Value alloca = emitAlloca(name.str(), ty, loc, align);
+ mlir::Location loc, const Twine &name,
+ bool insertIntoFnEntryBlock) {
+ mlir::Value alloca =
+ emitAlloca(name.str(), ty, loc, align, insertIntoFnEntryBlock);
return Address(alloca, ty, align);
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 16547f2401292..1df7767d6f2d7 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -138,7 +138,8 @@ mlir::Location CIRGenFunction::getLoc(mlir::Location lhs, mlir::Location rhs) {
void CIRGenFunction::emitAndUpdateRetAlloca(QualType type, mlir::Location loc,
CharUnits alignment) {
if (!type->isVoidType()) {
- fnRetAlloca = emitAlloca("__retval", convertType(type), loc, alignment);
+ fnRetAlloca = emitAlloca("__retval", convertType(type), loc, alignment,
+ /*insertIntoFnEntryBlock=*/false);
}
}
@@ -293,7 +294,8 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
mlir::Value addrVal =
emitAlloca(cast<NamedDecl>(paramVar)->getName(),
- convertType(paramVar->getType()), paramLoc, alignment);
+ convertType(paramVar->getType()), paramLoc, alignment,
+ /*insertIntoFnEntryBlock=*/false);
declare(addrVal, paramVar, paramVar->getType(), paramLoc, alignment,
/*isParam=*/true);
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 631217cf67762..1e6d7a44181c6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -111,7 +111,13 @@ class CIRGenFunction : public CIRGenTypeCache {
public:
mlir::Value emitAlloca(llvm::StringRef name, mlir::Type ty,
- mlir::Location loc, clang::CharUnits alignment);
+ mlir::Location loc, clang::CharUnits alignment,
+ bool insertIntoFnEntryBlock,
+ mlir::Value arraySize = nullptr);
+ mlir::Value emitAlloca(llvm::StringRef name, mlir::Type ty,
+ mlir::Location loc, clang::CharUnits alignment,
+ mlir::OpBuilder::InsertPoint ip,
+ mlir::Value arraySize = nullptr);
mlir::Value createDummyValue(mlir::Location loc, clang::QualType qt);
@@ -483,7 +489,7 @@ class CIRGenFunction : public CIRGenTypeCache {
LexicalScope *curLexScope = nullptr;
Address createTempAlloca(mlir::Type ty, CharUnits align, mlir::Location loc,
- const Twine &name = "tmp");
+ const Twine &name, bool insertIntoFnEntryBlock);
};
} // namespace clang::CIRGen
diff --git a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
index 648666d2461de..4678435b54c79 100644
--- a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
+++ b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
@@ -1,6 +1,7 @@
add_clang_library(MLIRCIRTransforms
CIRCanonicalize.cpp
FlattenCFG.cpp
+ HoistAllocas.cpp
DEPENDS
MLIRCIRPassIncGen
diff --git a/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp
new file mode 100644
index 0000000000000..3306e825f17f7
--- /dev/null
+++ b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp
@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PassDetail.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/Support/LogicalResult.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
+#include "clang/CIR/Dialect/Passes.h"
+#include "clang/CIR/MissingFeatures.h"
+#include "llvm/Support/TimeProfiler.h"
+
+using namespace mlir;
+using namespace cir;
+
+namespace {
+
+struct HoistAllocasPass : public HoistAllocasBase<HoistAllocasPass> {
+
+ HoistAllocasPass() = default;
+ void runOnOperation() override;
+};
+
+static void process(mlir::ModuleOp mod, cir::FuncOp func) {
+ if (func.getRegion().empty())
+ return;
+
+ // Hoist all static allocas to the entry block.
+ mlir::Block &entryBlock = func.getRegion().front();
+ llvm::SmallVector<cir::AllocaOp> allocas;
+ func.getBody().walk([&](cir::AllocaOp alloca) {
+ if (alloca->getBlock() == &entryBlock)
+ return;
+ // Don't hoist allocas with dynamic alloca size.
+ assert(!cir::MissingFeatures::opAllocaDynAllocSize());
+ allocas.push_back(alloca);
+ });
+ if (allocas.empty())
+ return;
+
+ mlir::Operation *insertPoint = &*entryBlock.begin();
+
+ for (cir::AllocaOp alloca : allocas) {
+ // Preserving the `const` attribute on hoisted allocas can cause LLVM to
+ // incorrectly introduce invariant group metadata in some circumstances.
+ // The incubator performs some analysis to determine whether the attribute
+ // can be preserved, but it only runs this analysis when optimizations are
+ // enabled. Until we start tracking the optimization level, we can just
+ // always remove the `const` attribute.
+ assert(!cir::MissingFeatures::optInfoAttr());
+ if (alloca.getConstant())
+ alloca.setConstant(false);
+
+ alloca->moveBefore(insertPoint);
+ }
+}
+
+void HoistAllocasPass::runOnOperation() {
+ llvm::TimeTraceScope scope("Hoist Allocas");
+ llvm::SmallVector<Operation *, 16> ops;
+
+ Operation *op = getOperation();
+ auto mod = mlir::dyn_cast<mlir::ModuleOp>(op);
+ if (!mod)
+ mod = op->getParentOfType<mlir::ModuleOp>();
+
+ getOperation()->walk([&](cir::FuncOp op) { process(mod, op); });
+}
+
+} // namespace
+
+std::unique_ptr<Pass> mlir::createHoistAllocasPass() {
+ return std::make_unique<HoistAllocasPass>();
+}
diff --git a/clang/lib/CIR/Lowering/CIRPasses.cpp b/clang/lib/CIR/Lowering/CIRPasses.cpp
index 1616ac6145151..a37a0480a56ac 100644
--- a/clang/lib/CIR/Lowering/CIRPasses.cpp
+++ b/clang/lib/CIR/Lowering/CIRPasses.cpp
@@ -37,6 +37,7 @@ mlir::LogicalResult runCIRToCIRPasses(mlir::ModuleOp theModule,
namespace mlir {
void populateCIRPreLoweringPasses(OpPassManager &pm) {
+ pm.addPass(createHoistAllocasPass());
pm.addPass(createCIRFlattenCFGPass());
}
diff --git a/clang/test/CIR/CodeGen/loop.cpp b/clang/test/CIR/CodeGen/loop.cpp
index 449317016e99d..61d18684ed1b2 100644
--- a/clang/test/CIR/CodeGen/loop.cpp
+++ b/clang/test/CIR/CodeGen/loop.cpp
@@ -44,3 +44,100 @@ void l0() {
// OGCG: br label %[[FOR_COND:.*]]
// OGCG: [[FOR_COND]]:
// OGCG: br label %[[FOR_COND]]
+
+void l1() {
+ for (int i = 0; ; ) {
+ }
+}
+
+// CIR: cir.func @l1
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+// CIR-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+// CIR-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT: cir.for : cond {
+// CIR-NEXT: %[[TRUE:.*]] = cir.const #true
+// CIR-NEXT: cir.condition(%[[TRUE]])
+// CIR-NEXT: } body {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: } step {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: }
+// CIR-NEXT: }
+// CIR-NEXT: cir.return
+// CIR-NEXT: }
+
+// LLVM: define void @l1()
+// LLVM: %[[I:.*]] = alloca i32, i64 1, align 4
+// LLVM: br label %[[LABEL1:.*]]
+// LLVM: [[LABEL1]]:
+// LLVM: store i32 0, ptr %[[I]], align 4
+// LLVM: br label %[[LABEL2:.*]]
+// LLVM: [[LABEL2]]:
+// LLVM: br i1 true, label %[[LABEL3:.*]], label %[[LABEL5:.*]]
+// LLVM: [[LABEL3]]:
+// LLVM: br label %[[LABEL4:.*]]
+// LLVM: [[LABEL4]]:
+// LLVM: br label %[[LABEL2]]
+// LLVM: [[LABEL5]]:
+// LLVM: br label %[[LABEL6:.*]]
+// LLVM: [[LABEL6]]:
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z2l1v()
+// OGCG: entry:
+// OGCG: %[[I:.*]] = alloca i32, align 4
+// OGCG: store i32 0, ptr %[[I]], align 4
+// OGCG: br label %[[FOR_COND:.*]]
+// OGCG: [[FOR_COND]]:
+// OGCG: br label %[[FOR_COND]]
+
+void l2() {
+ for (;;) {
+ int i = 0;
+ }
+}
+
+// CIR: cir.func @l2
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: cir.for : cond {
+// CIR-NEXT: %[[TRUE:.*]] = cir.const #true
+// CIR-NEXT: cir.condition(%[[TRUE]])
+// CIR-NEXT: } body {
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+// CIR-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+// CIR-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT: }
+// CIR-NEXT: cir.yield
+// CIR-NEXT: } step {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: }
+// CIR-NEXT: }
+// CIR-NEXT: cir.return
+// CIR-NEXT: }
+
+// LLVM: define void @l2()
+// LLVM: %[[I:.*]] = alloca i32, i64 1, align 4
+// LLVM: br label %[[LABEL1:.*]]
+// LLVM: [[LABEL1]]:
+// LLVM: br label %[[LABEL2:.*]]
+// LLVM: [[LABEL2]]:
+// LLVM: br i1 true, label %[[LABEL3:.*]], label %[[LABEL5:.*]]
+// LLVM: [[LABEL3]]:
+// LLVM: store i32 0, ptr %[[I]], align 4
+// LLVM: br label %[[LABEL4:.*]]
+// LLVM: [[LABEL4]]:
+// LLVM: br label %[[LABEL2]]
+// LLVM: [[LABEL5]]:
+// LLVM: br label %[[LABEL6:.*]]
+// LLVM: [[LABEL6]]:
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z2l2v()
+// OGCG: entry:
+// OGCG: %[[I:.*]] = alloca i32, align 4
+// OGCG: br label %[[FOR_COND:.*]]
+// OGCG: [[FOR_COND]]:
+// OGCG: store i32 0, ptr %[[I]], align 4
+// OGCG: br label %[[FOR_COND]]
diff --git a/clang/test/CIR/Transforms/hoist-allocas.cir b/clang/test/CIR/Transforms/hoist-allocas.cir
new file mode 100644
index 0000000000000..df7b9f48be9dc
--- /dev/null
+++ b/clang/test/CIR/Transforms/hoist-allocas.cir
@@ -0,0 +1,113 @@
+ // RUN: cir-opt %s -cir-hoist-allocas -o - | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+#true = #cir.bool<true> : !cir.bool
+
+module {
+ cir.func @l1() {
+ cir.scope {
+ %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ %1 = cir.const #cir.int<0> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.for : cond {
+ %2 = cir.const #true
+ cir.condition(%2)
+ } body {
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l1
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+
+ cir.func @l2() {
+ cir.scope {
+ cir.for : cond {
+ %0 = cir.const #true
+ cir.condition(%0)
+ } body {
+ cir.scope {
+ %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ %2 = cir.const #cir.int<0> : !s32i
+ cir.store %2, %1 : !s32i, !cir.ptr<!s32i>
+ }
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l2
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+
+ cir.func @l3() {
+ cir.scope {
+ cir.for : cond {
+ %0 = cir.const #true
+ cir.condition(%0)
+ } body {
+ cir.scope {
+ %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init, const] {alignment = 4 : i64}
+ %2 = cir.const #cir.int<0> : !s32i
+ cir.store %2, %1 : !s32i, !cir.ptr<!s32i>
+ }
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l3
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+}
diff --git a/clang/tools/cir-opt/cir-opt.cpp b/clang/tools/cir-opt/cir-opt.cpp
index 79a26c7986f0b..e50fa70582966 100644
--- a/clang/tools/cir-opt/cir-opt.cpp
+++ b/clang/tools/cir-opt/cir-opt.cpp
@@ -48,6 +48,10 @@ int main(int argc, char **argv) {
return mlir::createCIRFlattenCFGPass();
});
+ ::mlir::registerPass([]() -> std::unique_ptr<::mlir::Pass> {
+ return mlir::createHoistAllocasPass();
+ });
+
mlir::registerTransformsPasses();
return mlir::asMainReturnCode(MlirOptMain(
|
Member
|
@llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesAlloca operations were being emitted into the entry block of the current Full diff: https://github.com/llvm/llvm-project/pull/132468.diff 13 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Passes.h b/clang/include/clang/CIR/Dialect/Passes.h
index aa84241bdecf0..133eb462dcf1f 100644
--- a/clang/include/clang/CIR/Dialect/Passes.h
+++ b/clang/include/clang/CIR/Dialect/Passes.h
@@ -22,6 +22,7 @@ namespace mlir {
std::unique_ptr<Pass> createCIRCanonicalizePass();
std::unique_ptr<Pass> createCIRFlattenCFGPass();
+std::unique_ptr<Pass> createHoistAllocasPass();
void populateCIRPreLoweringPasses(mlir::OpPassManager &pm);
diff --git a/clang/include/clang/CIR/Dialect/Passes.td b/clang/include/clang/CIR/Dialect/Passes.td
index 16133d020a7c8..74c255861c879 100644
--- a/clang/include/clang/CIR/Dialect/Passes.td
+++ b/clang/include/clang/CIR/Dialect/Passes.td
@@ -29,6 +29,16 @@ def CIRCanonicalize : Pass<"cir-canonicalize"> {
let dependentDialects = ["cir::CIRDialect"];
}
+def HoistAllocas : Pass<"cir-hoist-allocas"> {
+ let summary = "Hoist allocas to the entry of the function";
+ let description = [{
+ This pass hoist all non-dynamic allocas to the entry of the function.
+ This is helpful for later code generation.
+ }];
+ let constructor = "mlir::createHoistAllocasPass()";
+ let dependentDialects = ["cir::CIRDialect"];
+}
+
def CIRFlattenCFG : Pass<"cir-flatten-cfg"> {
let summary = "Produces flatten CFG";
let description = [{
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 3e33e5dc60194..3f2e56a43fbef 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -127,6 +127,9 @@ struct MissingFeatures {
static bool ternaryOp() { return false; }
static bool tryOp() { return false; }
static bool zextOp() { return false; }
+
+ // Future CIR attributes
+ static bool optInfoAttr() { return false; }
};
} // namespace cir
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index a93e8dbcb42de..f2153c23ebb43 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -49,7 +49,8 @@ CIRGenFunction::emitAutoVarAlloca(const VarDecl &d) {
// A normal fixed sized variable becomes an alloca in the entry block,
mlir::Type allocaTy = convertTypeForMem(ty);
// Create the temp alloca and declare variable using it.
- address = createTempAlloca(allocaTy, alignment, loc, d.getName());
+ address = createTempAlloca(allocaTy, alignment, loc, d.getName(),
+ /*insertIntoFnEntryBlock=*/false);
declare(address.getPointer(), &d, ty, getLoc(d.getSourceRange()), alignment);
emission.Addr = address;
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 306130b80d457..d16480143c47e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -261,10 +261,27 @@ void CIRGenFunction::emitIgnoredExpr(const Expr *e) {
}
mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
- mlir::Location loc,
- CharUnits alignment) {
- mlir::Block *entryBlock = getCurFunctionEntryBlock();
+ mlir::Location loc, CharUnits alignment,
+ bool insertIntoFnEntryBlock,
+ mlir::Value arraySize) {
+ mlir::Block *entryBlock = insertIntoFnEntryBlock
+ ? getCurFunctionEntryBlock()
+ : curLexScope->getEntryBlock();
+
+ // If this is an alloca in the entry basic block of a cir.try and there's
+ // a surrounding cir.scope, make sure the alloca ends up in the surrounding
+ // scope instead. This is necessary in order to guarantee all SSA values are
+ // reachable during cleanups.
+ assert(!cir::MissingFeatures::tryOp());
+
+ return emitAlloca(name, ty, loc, alignment,
+ builder.getBestAllocaInsertPoint(entryBlock), arraySize);
+}
+mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
+ mlir::Location loc, CharUnits alignment,
+ mlir::OpBuilder::InsertPoint ip,
+ mlir::Value arraySize) {
// CIR uses its own alloca address space rather than follow the target data
// layout like original CodeGen. The data layout awareness should be done in
// the lowering pass instead.
@@ -275,7 +292,7 @@ mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
mlir::Value addr;
{
mlir::OpBuilder::InsertionGuard guard(builder);
- builder.restoreInsertionPoint(builder.getBestAllocaInsertPoint(entryBlock));
+ builder.restoreInsertionPoint(ip);
addr = builder.createAlloca(loc, /*addr type*/ localVarPtrTy,
/*var type*/ ty, name, alignIntAttr);
assert(!cir::MissingFeatures::astVarDeclInterface());
@@ -290,11 +307,13 @@ mlir::Value CIRGenFunction::createDummyValue(mlir::Location loc,
return builder.createDummyValue(loc, t, alignment);
}
-/// This creates an alloca and inserts it at the current insertion point of the
-/// builder.
+/// This creates an alloca and inserts it into the entry block if
+/// \p insertIntoFnEntryBlock is true, otherwise it inserts it at the current
+/// insertion point of the builder.
Address CIRGenFunction::createTempAlloca(mlir::Type ty, CharUnits align,
- mlir::Location loc,
- const Twine &name) {
- mlir::Value alloca = emitAlloca(name.str(), ty, loc, align);
+ mlir::Location loc, const Twine &name,
+ bool insertIntoFnEntryBlock) {
+ mlir::Value alloca =
+ emitAlloca(name.str(), ty, loc, align, insertIntoFnEntryBlock);
return Address(alloca, ty, align);
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 16547f2401292..1df7767d6f2d7 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -138,7 +138,8 @@ mlir::Location CIRGenFunction::getLoc(mlir::Location lhs, mlir::Location rhs) {
void CIRGenFunction::emitAndUpdateRetAlloca(QualType type, mlir::Location loc,
CharUnits alignment) {
if (!type->isVoidType()) {
- fnRetAlloca = emitAlloca("__retval", convertType(type), loc, alignment);
+ fnRetAlloca = emitAlloca("__retval", convertType(type), loc, alignment,
+ /*insertIntoFnEntryBlock=*/false);
}
}
@@ -293,7 +294,8 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
mlir::Value addrVal =
emitAlloca(cast<NamedDecl>(paramVar)->getName(),
- convertType(paramVar->getType()), paramLoc, alignment);
+ convertType(paramVar->getType()), paramLoc, alignment,
+ /*insertIntoFnEntryBlock=*/false);
declare(addrVal, paramVar, paramVar->getType(), paramLoc, alignment,
/*isParam=*/true);
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 631217cf67762..1e6d7a44181c6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -111,7 +111,13 @@ class CIRGenFunction : public CIRGenTypeCache {
public:
mlir::Value emitAlloca(llvm::StringRef name, mlir::Type ty,
- mlir::Location loc, clang::CharUnits alignment);
+ mlir::Location loc, clang::CharUnits alignment,
+ bool insertIntoFnEntryBlock,
+ mlir::Value arraySize = nullptr);
+ mlir::Value emitAlloca(llvm::StringRef name, mlir::Type ty,
+ mlir::Location loc, clang::CharUnits alignment,
+ mlir::OpBuilder::InsertPoint ip,
+ mlir::Value arraySize = nullptr);
mlir::Value createDummyValue(mlir::Location loc, clang::QualType qt);
@@ -483,7 +489,7 @@ class CIRGenFunction : public CIRGenTypeCache {
LexicalScope *curLexScope = nullptr;
Address createTempAlloca(mlir::Type ty, CharUnits align, mlir::Location loc,
- const Twine &name = "tmp");
+ const Twine &name, bool insertIntoFnEntryBlock);
};
} // namespace clang::CIRGen
diff --git a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
index 648666d2461de..4678435b54c79 100644
--- a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
+++ b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
@@ -1,6 +1,7 @@
add_clang_library(MLIRCIRTransforms
CIRCanonicalize.cpp
FlattenCFG.cpp
+ HoistAllocas.cpp
DEPENDS
MLIRCIRPassIncGen
diff --git a/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp
new file mode 100644
index 0000000000000..3306e825f17f7
--- /dev/null
+++ b/clang/lib/CIR/Dialect/Transforms/HoistAllocas.cpp
@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PassDetail.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/Support/LogicalResult.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
+#include "clang/CIR/Dialect/Passes.h"
+#include "clang/CIR/MissingFeatures.h"
+#include "llvm/Support/TimeProfiler.h"
+
+using namespace mlir;
+using namespace cir;
+
+namespace {
+
+struct HoistAllocasPass : public HoistAllocasBase<HoistAllocasPass> {
+
+ HoistAllocasPass() = default;
+ void runOnOperation() override;
+};
+
+static void process(mlir::ModuleOp mod, cir::FuncOp func) {
+ if (func.getRegion().empty())
+ return;
+
+ // Hoist all static allocas to the entry block.
+ mlir::Block &entryBlock = func.getRegion().front();
+ llvm::SmallVector<cir::AllocaOp> allocas;
+ func.getBody().walk([&](cir::AllocaOp alloca) {
+ if (alloca->getBlock() == &entryBlock)
+ return;
+ // Don't hoist allocas with dynamic alloca size.
+ assert(!cir::MissingFeatures::opAllocaDynAllocSize());
+ allocas.push_back(alloca);
+ });
+ if (allocas.empty())
+ return;
+
+ mlir::Operation *insertPoint = &*entryBlock.begin();
+
+ for (cir::AllocaOp alloca : allocas) {
+ // Preserving the `const` attribute on hoisted allocas can cause LLVM to
+ // incorrectly introduce invariant group metadata in some circumstances.
+ // The incubator performs some analysis to determine whether the attribute
+ // can be preserved, but it only runs this analysis when optimizations are
+ // enabled. Until we start tracking the optimization level, we can just
+ // always remove the `const` attribute.
+ assert(!cir::MissingFeatures::optInfoAttr());
+ if (alloca.getConstant())
+ alloca.setConstant(false);
+
+ alloca->moveBefore(insertPoint);
+ }
+}
+
+void HoistAllocasPass::runOnOperation() {
+ llvm::TimeTraceScope scope("Hoist Allocas");
+ llvm::SmallVector<Operation *, 16> ops;
+
+ Operation *op = getOperation();
+ auto mod = mlir::dyn_cast<mlir::ModuleOp>(op);
+ if (!mod)
+ mod = op->getParentOfType<mlir::ModuleOp>();
+
+ getOperation()->walk([&](cir::FuncOp op) { process(mod, op); });
+}
+
+} // namespace
+
+std::unique_ptr<Pass> mlir::createHoistAllocasPass() {
+ return std::make_unique<HoistAllocasPass>();
+}
diff --git a/clang/lib/CIR/Lowering/CIRPasses.cpp b/clang/lib/CIR/Lowering/CIRPasses.cpp
index 1616ac6145151..a37a0480a56ac 100644
--- a/clang/lib/CIR/Lowering/CIRPasses.cpp
+++ b/clang/lib/CIR/Lowering/CIRPasses.cpp
@@ -37,6 +37,7 @@ mlir::LogicalResult runCIRToCIRPasses(mlir::ModuleOp theModule,
namespace mlir {
void populateCIRPreLoweringPasses(OpPassManager &pm) {
+ pm.addPass(createHoistAllocasPass());
pm.addPass(createCIRFlattenCFGPass());
}
diff --git a/clang/test/CIR/CodeGen/loop.cpp b/clang/test/CIR/CodeGen/loop.cpp
index 449317016e99d..61d18684ed1b2 100644
--- a/clang/test/CIR/CodeGen/loop.cpp
+++ b/clang/test/CIR/CodeGen/loop.cpp
@@ -44,3 +44,100 @@ void l0() {
// OGCG: br label %[[FOR_COND:.*]]
// OGCG: [[FOR_COND]]:
// OGCG: br label %[[FOR_COND]]
+
+void l1() {
+ for (int i = 0; ; ) {
+ }
+}
+
+// CIR: cir.func @l1
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+// CIR-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+// CIR-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT: cir.for : cond {
+// CIR-NEXT: %[[TRUE:.*]] = cir.const #true
+// CIR-NEXT: cir.condition(%[[TRUE]])
+// CIR-NEXT: } body {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: } step {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: }
+// CIR-NEXT: }
+// CIR-NEXT: cir.return
+// CIR-NEXT: }
+
+// LLVM: define void @l1()
+// LLVM: %[[I:.*]] = alloca i32, i64 1, align 4
+// LLVM: br label %[[LABEL1:.*]]
+// LLVM: [[LABEL1]]:
+// LLVM: store i32 0, ptr %[[I]], align 4
+// LLVM: br label %[[LABEL2:.*]]
+// LLVM: [[LABEL2]]:
+// LLVM: br i1 true, label %[[LABEL3:.*]], label %[[LABEL5:.*]]
+// LLVM: [[LABEL3]]:
+// LLVM: br label %[[LABEL4:.*]]
+// LLVM: [[LABEL4]]:
+// LLVM: br label %[[LABEL2]]
+// LLVM: [[LABEL5]]:
+// LLVM: br label %[[LABEL6:.*]]
+// LLVM: [[LABEL6]]:
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z2l1v()
+// OGCG: entry:
+// OGCG: %[[I:.*]] = alloca i32, align 4
+// OGCG: store i32 0, ptr %[[I]], align 4
+// OGCG: br label %[[FOR_COND:.*]]
+// OGCG: [[FOR_COND]]:
+// OGCG: br label %[[FOR_COND]]
+
+void l2() {
+ for (;;) {
+ int i = 0;
+ }
+}
+
+// CIR: cir.func @l2
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: cir.for : cond {
+// CIR-NEXT: %[[TRUE:.*]] = cir.const #true
+// CIR-NEXT: cir.condition(%[[TRUE]])
+// CIR-NEXT: } body {
+// CIR-NEXT: cir.scope {
+// CIR-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+// CIR-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+// CIR-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT: }
+// CIR-NEXT: cir.yield
+// CIR-NEXT: } step {
+// CIR-NEXT: cir.yield
+// CIR-NEXT: }
+// CIR-NEXT: }
+// CIR-NEXT: cir.return
+// CIR-NEXT: }
+
+// LLVM: define void @l2()
+// LLVM: %[[I:.*]] = alloca i32, i64 1, align 4
+// LLVM: br label %[[LABEL1:.*]]
+// LLVM: [[LABEL1]]:
+// LLVM: br label %[[LABEL2:.*]]
+// LLVM: [[LABEL2]]:
+// LLVM: br i1 true, label %[[LABEL3:.*]], label %[[LABEL5:.*]]
+// LLVM: [[LABEL3]]:
+// LLVM: store i32 0, ptr %[[I]], align 4
+// LLVM: br label %[[LABEL4:.*]]
+// LLVM: [[LABEL4]]:
+// LLVM: br label %[[LABEL2]]
+// LLVM: [[LABEL5]]:
+// LLVM: br label %[[LABEL6:.*]]
+// LLVM: [[LABEL6]]:
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_Z2l2v()
+// OGCG: entry:
+// OGCG: %[[I:.*]] = alloca i32, align 4
+// OGCG: br label %[[FOR_COND:.*]]
+// OGCG: [[FOR_COND]]:
+// OGCG: store i32 0, ptr %[[I]], align 4
+// OGCG: br label %[[FOR_COND]]
diff --git a/clang/test/CIR/Transforms/hoist-allocas.cir b/clang/test/CIR/Transforms/hoist-allocas.cir
new file mode 100644
index 0000000000000..df7b9f48be9dc
--- /dev/null
+++ b/clang/test/CIR/Transforms/hoist-allocas.cir
@@ -0,0 +1,113 @@
+ // RUN: cir-opt %s -cir-hoist-allocas -o - | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+#true = #cir.bool<true> : !cir.bool
+
+module {
+ cir.func @l1() {
+ cir.scope {
+ %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ %1 = cir.const #cir.int<0> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.for : cond {
+ %2 = cir.const #true
+ cir.condition(%2)
+ } body {
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l1
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+
+ cir.func @l2() {
+ cir.scope {
+ cir.for : cond {
+ %0 = cir.const #true
+ cir.condition(%0)
+ } body {
+ cir.scope {
+ %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ %2 = cir.const #cir.int<0> : !s32i
+ cir.store %2, %1 : !s32i, !cir.ptr<!s32i>
+ }
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l2
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+
+ cir.func @l3() {
+ cir.scope {
+ cir.for : cond {
+ %0 = cir.const #true
+ cir.condition(%0)
+ } body {
+ cir.scope {
+ %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init, const] {alignment = 4 : i64}
+ %2 = cir.const #cir.int<0> : !s32i
+ cir.store %2, %1 : !s32i, !cir.ptr<!s32i>
+ }
+ cir.yield
+ } step {
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ // CHECK: cir.func @l3
+ // CHECK-NEXT: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: cir.for : cond {
+ // CHECK-NEXT: %[[TRUE:.*]] = cir.const #true
+ // CHECK-NEXT: cir.condition(%[[TRUE]])
+ // CHECK-NEXT: } body {
+ // CHECK-NEXT: cir.scope {
+ // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.store %[[ZERO]], %[[I]] : !s32i, !cir.ptr<!s32i>
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: } step {
+ // CHECK-NEXT: cir.yield
+ // CHECK-NEXT: }
+ // CHECK-NEXT: }
+ // CHECK-NEXT: cir.return
+ // CHECK-NEXT: }
+}
diff --git a/clang/tools/cir-opt/cir-opt.cpp b/clang/tools/cir-opt/cir-opt.cpp
index 79a26c7986f0b..e50fa70582966 100644
--- a/clang/tools/cir-opt/cir-opt.cpp
+++ b/clang/tools/cir-opt/cir-opt.cpp
@@ -48,6 +48,10 @@ int main(int argc, char **argv) {
return mlir::createCIRFlattenCFGPass();
});
+ ::mlir::registerPass([]() -> std::unique_ptr<::mlir::Pass> {
+ return mlir::createHoistAllocasPass();
+ });
+
mlir::registerTransformsPasses();
return mlir::asMainReturnCode(MlirOptMain(
|
mmha
approved these changes
Mar 24, 2025
| emitAlloca(cast<NamedDecl>(paramVar)->getName(), | ||
| convertType(paramVar->getType()), paramLoc, alignment); | ||
| convertType(paramVar->getType()), paramLoc, alignment, | ||
| /*insertIntoFnEntryBlock=*/false); |
Contributor
There was a problem hiding this comment.
It doesn't really make a difference because this is called by startFunction but shouldn't the return value always be in the function entry block?
Member
There was a problem hiding this comment.
I guess here it won't make a difference but for consistency maybe better to leave the default as true? Leaving it as false can be confusing as to the intent (even tho in practice won't make a difference).
Contributor
Author
There was a problem hiding this comment.
This is an optional argument in the incubator, and it defaults to false there. However, in the incubator, this call happens inside of declare so we don't know there that it's going into the entry block and instead rely on the location having been set.
This should always insert into the entry block, because we set the insertion point above. If I set this to true, emitAlloca will go through the process of finding the correct insertion point, saving, setting, and restoring, for each parameter value.
It's probably better to get rid of the insertIntoFnEntyBlock argument altogether and add a separate emitAllocaIntoFnEntryBlock function for the cases where that is needed.
Contributor
Author
There was a problem hiding this comment.
On second thought, I can just change this to true as suggested. I was thinking of the call to `getBestAllocaInsertPoint, but I see that happens regardless of where we're inserting.
Member
There was a problem hiding this comment.
I guess here it won't make a difference but for consistency maybe better to leave the default as true?
Sorry, by default I meant the value for the callsite, not the declaration!
| // OGCG: br label %[[FOR_COND]] | ||
|
|
||
| void l2() { | ||
| for (;;) { |
Contributor
There was a problem hiding this comment.
Can you add another test without any braces so we have a for loop body without a CompoundStmt?
| // Hoist all static allocas to the entry block. | ||
| mlir::Block &entryBlock = func.getRegion().front(); | ||
| llvm::SmallVector<cir::AllocaOp> allocas; | ||
| func.getBody().walk([&](cir::AllocaOp alloca) { |
Contributor
There was a problem hiding this comment.
You create a SmallVector of AllocaOps here only to iterate that list again right below.
Can you merge these two loops? It would save us the creation of that vector.
Contributor
Author
There was a problem hiding this comment.
I'm not sure I can do that because the loop below is moving allocas. Hoisting them midwalk could disrupt the walking process.
Contributor
There was a problem hiding this comment.
As far as I know walk traverses post order, so the entry block should come last. I don't know what happens within a block, but then again you're also modifying the list in the second loop.
But I don't know for sure if it works.
Contributor
Author
There was a problem hiding this comment.
I'll give this a try. The documentation for mlir::Block::walk says that I am allowed to erase operations from the block.
bcardosolopes
approved these changes
Mar 24, 2025
| emitAlloca(cast<NamedDecl>(paramVar)->getName(), | ||
| convertType(paramVar->getType()), paramLoc, alignment); | ||
| convertType(paramVar->getType()), paramLoc, alignment, | ||
| /*insertIntoFnEntryBlock=*/false); |
Member
There was a problem hiding this comment.
I guess here it won't make a difference but for consistency maybe better to leave the default as true? Leaving it as false can be confusing as to the intent (even tho in practice won't make a difference).
xlauko
reviewed
Mar 25, 2025
Contributor
There was a problem hiding this comment.
There is probably no need to do recursive walk here? @bcardosolopes can cir::FuncOp have nested cir::FuncOp?
If not this should probably be PreOrder walk and return WalkResult::skip() from the lambda to not walk its body.
Contributor
Author
There was a problem hiding this comment.
It looks like the incubator is generating lambdas as separate functions. I'll give the suggested change a try.
Contributor
There was a problem hiding this comment.
A thing to look out for here and we don't have implemented in the incubator yet are ObjC/OpenCL blocks.
Member
There was a problem hiding this comment.
There is probably no need to do recursive walk here?
You probably right
can cir::FuncOp have nested cir::FuncOp?
nope!
Alloca operations were being emitted into the entry block of the current function unconditionally, even if the variable they represented was declared in a different scope. This change upstreams the code for handling insertion of the alloca into the proper lexcial scope. It also adds a CIR-to-CIR transformation to hoist allocas to the function entry block, which is necessary to produce the expected LLVM IR during lowering.
andykaylor
force-pushed
the
cir-fix-loop-alloca
branch
from
322ebce to
3ac208b
Compare
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14928 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Alloca operations were being emitted into the entry block of the current
function unconditionally, even if the variable they represented was
declared in a different scope. This change upstreams the code for handling
insertion of the alloca into the proper lexcial scope. It also adds a
CIR-to-CIR transformation to hoist allocas to the function entry block,
which is necessary to produce the expected LLVM IR during lowering.