[CIR] Implement Statement Expressions #153677
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
mmha
requested review from
lanza,
bcardosolopes,
xlauko and
andykaylor
as code owners
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Morris Hafner (mmha) ChangesDepends on #153625 This patch adds support for statement expressions. It also changes emitCompoundStmt and emitCompoundStmtWithoutScope to accept an Address that the optional result is written to. This allows the creation of the alloca ahead of the creation of the scope which saves us from hoisting the alloca to its parent scope. Full diff: https://github.com/llvm/llvm-project/pull/153677.diff 8 Files Affected:
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 805c43e6d5054..baab62f572b98 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -27,9 +27,6 @@ struct MissingFeatures {
// Address space related
static bool addressSpace() { return false; }
- // CIRGenFunction implementation details
- static bool cgfSymbolTable() { return false; }
-
// Unhandled global/linkage information.
static bool opGlobalThreadLocal() { return false; }
static bool opGlobalConstant() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index cc1fd81433b53..8bcca6f5d1803 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -591,6 +591,12 @@ LValue CIRGenFunction::emitDeclRefLValue(const DeclRefExpr *e) {
? emitLoadOfReferenceLValue(addr, getLoc(e->getSourceRange()),
vd->getType(), AlignmentSource::Decl)
: makeAddrLValue(addr, ty, AlignmentSource::Decl);
+
+ // Statics are defined as globals, so they are not include in the function's
+ // symbol table.
+ assert((vd->isStaticLocal() || symbolTable.count(vd)) &&
+ "non-static locals should be already mapped");
+
return lv;
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
index 6b6ac701e6867..9d85aacb76ee0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
@@ -69,6 +69,12 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
void Visit(Expr *e) { StmtVisitor<AggExprEmitter>::Visit(e); }
void VisitCallExpr(const CallExpr *e);
+ void VisitStmtExpr(const StmtExpr *e) {
+ CIRGenFunction::StmtExprEvaluation eval(cgf);
+ Address retAlloca =
+ cgf.createMemTemp(e->getType(), cgf.getLoc(e->getSourceRange()));
+ cgf.emitCompoundStmt(*e->getSubStmt(), &retAlloca, dest);
+ }
void VisitDeclRefExpr(DeclRefExpr *e) { emitAggLoadOfLValue(e); }
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 8649bab91ce8e..e577175aada18 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -185,6 +185,21 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
mlir::Value VisitCastExpr(CastExpr *e);
mlir::Value VisitCallExpr(const CallExpr *e);
+ mlir::Value VisitStmtExpr(StmtExpr *e) {
+ CIRGenFunction::StmtExprEvaluation eval(cgf);
+ if (e->getType()->isVoidType()) {
+ cgf.emitCompoundStmt(*e->getSubStmt());
+ return {};
+ }
+
+ Address retAlloca =
+ cgf.createMemTemp(e->getType(), cgf.getLoc(e->getSourceRange()));
+ cgf.emitCompoundStmt(*e->getSubStmt(), &retAlloca);
+
+ return cgf.emitLoadOfScalar(cgf.makeAddrLValue(retAlloca, e->getType()),
+ e->getExprLoc());
+ }
+
mlir::Value VisitArraySubscriptExpr(ArraySubscriptExpr *e) {
if (e->getBase()->getType()->isVectorType()) {
assert(!cir::MissingFeatures::scalableVectors());
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 8a3f5ab78ab59..d6a0792292604 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -215,7 +215,7 @@ void CIRGenFunction::declare(mlir::Value addrVal, const Decl *var, QualType ty,
mlir::Location loc, CharUnits alignment,
bool isParam) {
assert(isa<NamedDecl>(var) && "Needs a named decl");
- assert(!cir::MissingFeatures::cgfSymbolTable());
+ assert(!symbolTable.count(var) && "not supposed to be available just yet");
auto allocaOp = addrVal.getDefiningOp<cir::AllocaOp>();
assert(allocaOp && "expected cir::AllocaOp");
@@ -224,6 +224,8 @@ void CIRGenFunction::declare(mlir::Value addrVal, const Decl *var, QualType ty,
allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
if (ty->isReferenceType() || ty.isConstQualified())
allocaOp.setConstantAttr(mlir::UnitAttr::get(&getMLIRContext()));
+
+ symbolTable.insert(var, allocaOp);
}
void CIRGenFunction::LexicalScope::cleanup() {
@@ -485,6 +487,9 @@ void CIRGenFunction::finishFunction(SourceLocation endLoc) {
}
mlir::LogicalResult CIRGenFunction::emitFunctionBody(const clang::Stmt *body) {
+ // We start with function level scope for variables.
+ SymTableScopeTy varScope(symbolTable);
+
auto result = mlir::LogicalResult::success();
if (const CompoundStmt *block = dyn_cast<CompoundStmt>(body))
emitCompoundStmtWithoutScope(*block);
@@ -531,6 +536,8 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
FunctionArgList args;
QualType retTy = buildFunctionArgList(gd, args);
+ // Create a scope in the symbol table to hold variable declarations.
+ SymTableScopeTy varScope(symbolTable);
{
LexicalScope lexScope(*this, fusedLoc, entryBB);
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index ddc1edd77010c..1ef82a2edf9d3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -31,6 +31,7 @@
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/MissingFeatures.h"
#include "clang/CIR/TypeEvaluationKind.h"
+#include "llvm/ADT/ScopedHashTable.h"
namespace {
class ScalarExprEmitter;
@@ -103,6 +104,14 @@ class CIRGenFunction : public CIRGenTypeCache {
/// Sanitizers enabled for this function.
clang::SanitizerSet sanOpts;
+ /// The symbol table maps a variable name to a value in the current scope.
+ /// Entering a function creates a new scope, and the function arguments are
+ /// added to the mapping. When the processing of a function is terminated,
+ /// the scope is destroyed and the mappings created in this scope are
+ /// dropped.
+ using SymTableTy = llvm::ScopedHashTable<const clang::Decl *, mlir::Value>;
+ SymTableTy symbolTable;
+
/// Whether or not a Microsoft-style asm block has been processed within
/// this fuction. These can potentially set the return value.
bool sawAsmBlock = false;
@@ -325,6 +334,9 @@ class CIRGenFunction : public CIRGenTypeCache {
~SourceLocRAIIObject() { restore(); }
};
+ using SymTableScopeTy =
+ llvm::ScopedHashTableScope<const clang::Decl *, mlir::Value>;
+
/// Hold counters for incrementally naming temporaries
unsigned counterRefTmp = 0;
unsigned counterAggTmp = 0;
@@ -499,7 +511,11 @@ class CIRGenFunction : public CIRGenTypeCache {
void setAddrOfLocalVar(const clang::VarDecl *vd, Address addr) {
assert(!localDeclMap.count(vd) && "Decl already exists in LocalDeclMap!");
localDeclMap.insert({vd, addr});
- // TODO: Add symbol table support
+
+ // Add to the symbol table if not there already.
+ if (symbolTable.count(vd))
+ return;
+ symbolTable.insert(vd, addr.getPointer());
}
bool shouldNullCheckClassCastValue(const CastExpr *ce);
@@ -1141,9 +1157,14 @@ class CIRGenFunction : public CIRGenTypeCache {
LValue emitScalarCompoundAssignWithComplex(const CompoundAssignOperator *e,
mlir::Value &result);
- void emitCompoundStmt(const clang::CompoundStmt &s);
+ Address emitCompoundStmt(const clang::CompoundStmt &s,
+ Address *lastValue = nullptr,
+ AggValueSlot slot = AggValueSlot::ignored());
- void emitCompoundStmtWithoutScope(const clang::CompoundStmt &s);
+ Address
+ emitCompoundStmtWithoutScope(const clang::CompoundStmt &s,
+ Address *lastValue = nullptr,
+ AggValueSlot slot = AggValueSlot::ignored());
void emitDecl(const clang::Decl &d, bool evaluateConditionDecl = false);
mlir::LogicalResult emitDeclStmt(const clang::DeclStmt &s);
@@ -1380,6 +1401,27 @@ class CIRGenFunction : public CIRGenTypeCache {
// we know if a temporary should be destroyed conditionally.
ConditionalEvaluation *outermostConditional = nullptr;
+ /// An RAII object to record that we're evaluating a statement
+ /// expression.
+ class StmtExprEvaluation {
+ CIRGenFunction &cgf;
+
+ /// We have to save the outermost conditional: cleanups in a
+ /// statement expression aren't conditional just because the
+ /// StmtExpr is.
+ ConditionalEvaluation *savedOutermostConditional;
+
+ public:
+ StmtExprEvaluation(CIRGenFunction &cgf)
+ : cgf(cgf), savedOutermostConditional(cgf.outermostConditional) {
+ cgf.outermostConditional = nullptr;
+ }
+
+ ~StmtExprEvaluation() {
+ cgf.outermostConditional = savedOutermostConditional;
+ }
+ };
+
template <typename FuncTy>
ConditionalInfo emitConditionalBlocks(const AbstractConditionalOperator *e,
const FuncTy &branchGenFunc);
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index dffe8b408b6da..92cce9c5f9ba5 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -14,6 +14,7 @@
#include "CIRGenFunction.h"
#include "mlir/IR/Builders.h"
+#include "mlir/IR/Location.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtOpenACC.h"
@@ -23,28 +24,79 @@ using namespace clang;
using namespace clang::CIRGen;
using namespace cir;
-void CIRGenFunction::emitCompoundStmtWithoutScope(const CompoundStmt &s) {
- for (auto *curStmt : s.body()) {
- if (emitStmt(curStmt, /*useCurrentScope=*/false).failed())
- getCIRGenModule().errorNYI(curStmt->getSourceRange(),
- std::string("emitCompoundStmtWithoutScope: ") +
- curStmt->getStmtClassName());
+Address CIRGenFunction::emitCompoundStmtWithoutScope(const CompoundStmt &s,
+ Address *lastValue,
+ AggValueSlot slot) {
+ const Stmt *exprResult = s.getStmtExprResult();
+ assert((!lastValue || (lastValue && exprResult)) &&
+ "If lastValue is not null then the CompoundStmt must have a "
+ "StmtExprResult");
+
+ Address retAlloca = Address::invalid();
+
+ for (Stmt *curStmt : s.body()) {
+ // We have to special case labels here. They are statements, but when put
+ // at the end of a statement expression, they yield the value of their
+ // subexpression. Handle this by walking through all labels we encounter,
+ // emitting them before we evaluate the subexpr.
+ // Similar issues arise for attributed statements.
+ if (lastValue && exprResult == curStmt) {
+ while (!isa<Expr>(exprResult)) {
+ if (const auto *ls = dyn_cast<LabelStmt>(exprResult)) {
+ if (emitLabel(*ls->getDecl()).failed())
+ return Address::invalid();
+ exprResult = ls->getSubStmt();
+ } else if (const auto *as = dyn_cast<AttributedStmt>(exprResult)) {
+ // FIXME: Update this if we ever have attributes that affect the
+ // semantics of an expression.
+ exprResult = as->getSubStmt();
+ } else {
+ llvm_unreachable("Unknown value statement");
+ }
+ }
+
+ const Expr *e = cast<Expr>(exprResult);
+ QualType exprTy = e->getType();
+ if (hasAggregateEvaluationKind(exprTy)) {
+ emitAggExpr(e, slot);
+ } else {
+ // We can't return an RValue here because there might be cleanups at
+ // the end of the StmtExpr. Because of that, we have to emit the result
+ // here into a temporary alloca.
+ emitAnyExprToMem(e, *lastValue, Qualifiers(),
+ /*IsInit*/ false);
+ }
+ } else {
+ if (emitStmt(curStmt, /*useCurrentScope=*/false).failed())
+ return Address::invalid();
+ }
}
+
+ return retAlloca;
}
-void CIRGenFunction::emitCompoundStmt(const CompoundStmt &s) {
+Address CIRGenFunction::emitCompoundStmt(const CompoundStmt &s,
+ Address *lastValue,
+ AggValueSlot slot) {
+ Address retAlloca = Address::invalid();
+
+ // Add local scope to track new declared variables.
+ SymTableScopeTy varScope(symbolTable);
mlir::Location scopeLoc = getLoc(s.getSourceRange());
mlir::OpBuilder::InsertPoint scopeInsPt;
builder.create<cir::ScopeOp>(
- scopeLoc, [&](mlir::OpBuilder &b, mlir::Type &type, mlir::Location loc) {
+ scopeLoc, /*scopeBuilder=*/
+ [&](mlir::OpBuilder &b, mlir::Type &type, mlir::Location loc) {
scopeInsPt = b.saveInsertionPoint();
});
{
mlir::OpBuilder::InsertionGuard guard(builder);
builder.restoreInsertionPoint(scopeInsPt);
- LexicalScope lexScope(*this, scopeLoc, builder.getInsertionBlock());
- emitCompoundStmtWithoutScope(s);
+ LexicalScope lexScope{*this, scopeLoc, builder.getInsertionBlock()};
+ retAlloca = emitCompoundStmtWithoutScope(s, lastValue, slot);
}
+
+ return retAlloca;
}
void CIRGenFunction::emitStopPoint(const Stmt *s) {
diff --git a/clang/test/CIR/CodeGen/statement-exprs.c b/clang/test/CIR/CodeGen/statement-exprs.c
new file mode 100644
index 0000000000000..67927bbcc1bdc
--- /dev/null
+++ b/clang/test/CIR/CodeGen/statement-exprs.c
@@ -0,0 +1,166 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+int f19(void) {
+ return ({ 3;;4;; });
+}
+
+// CIR: cir.func dso_local @f19() -> !s32i
+// CIR: %[[RETVAL:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"]
+// CIR: %[[TMP:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["tmp"]
+// CIR: cir.scope {
+// CIR: %[[C3:.+]] = cir.const #cir.int<3> : !s32i
+// CIR: %[[C4:.+]] = cir.const #cir.int<4> : !s32i
+// CIR: cir.store {{.*}} %[[C4]], %[[TMP]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+// CIR: %[[TMP_VAL:.+]] = cir.load {{.*}} %[[TMP]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.store %[[TMP_VAL]], %[[RETVAL]] : !s32i, !cir.ptr<!s32i>
+// CIR: %[[RES:.+]] = cir.load %[[RETVAL]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.return %[[RES]] : !s32i
+
+// LLVM: define dso_local i32 @f19()
+// LLVM: %[[VAR1:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR2:.+]] = alloca i32, i64 1
+// LLVM: br label %[[LBL3:.+]]
+// LLVM: [[LBL3]]:
+// LLVM: store i32 4, ptr %[[VAR2]]
+// LLVM: br label %[[LBL4:.+]]
+// LLVM: [[LBL4]]:
+// LLVM: %[[V1:.+]] = load i32, ptr %[[VAR2]]
+// LLVM: store i32 %[[V1]], ptr %[[VAR1]]
+// LLVM: %[[RES:.+]] = load i32, ptr %[[VAR1]]
+// LLVM: ret i32 %[[RES]]
+
+// OGCG: define dso_local i32 @f19()
+// OGCG: entry:
+// OGCG: %[[TMP:.+]] = alloca i32
+// OGCG: store i32 4, ptr %[[TMP]]
+// OGCG: %[[TMP_VAL:.+]] = load i32, ptr %[[TMP]]
+// OGCG: ret i32 %[[TMP_VAL]]
+
+
+int nested(void) {
+ ({123;});
+ {
+ int bar = 987;
+ return ({ ({ int asdf = 123; asdf; }); ({9999;}); });
+ }
+}
+
+// CIR: cir.func dso_local @nested() -> !s32i
+// CIR: %[[RETVAL:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"]
+// CIR: %[[TMP_OUTER:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["tmp"]
+// CIR: cir.scope {
+// CIR: %[[C123_OUTER:.+]] = cir.const #cir.int<123> : !s32i
+// CIR: cir.store {{.*}} %[[C123_OUTER]], %[[TMP_OUTER]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+// CIR: %[[LOAD_TMP_OUTER:.+]] = cir.load {{.*}} %[[TMP_OUTER]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.scope {
+// CIR: %[[BAR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["bar", init]
+// CIR: %[[TMP_BARRET:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["tmp"]
+// CIR: %[[C987:.+]] = cir.const #cir.int<987> : !s32i
+// CIR: cir.store {{.*}} %[[C987]], %[[BAR]] : !s32i, !cir.ptr<!s32i>
+// CIR: cir.scope {
+// CIR: %[[TMP1:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["tmp"]
+// CIR: %[[TMP2:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["tmp"]
+// CIR: cir.scope {
+// CIR: %[[ASDF:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["asdf", init]
+// CIR: %[[C123_INNER:.+]] = cir.const #cir.int<123> : !s32i
+// CIR: cir.store {{.*}} %[[C123_INNER]], %[[ASDF]] : !s32i, !cir.ptr<!s32i>
+// CIR: %[[LOAD_ASDF:.+]] = cir.load {{.*}} %[[ASDF]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.store {{.*}} %[[LOAD_ASDF]], %[[TMP1]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+// CIR: %[[V1:.+]] = cir.load {{.*}} %[[TMP1]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.scope {
+// CIR: %[[C9999:.+]] = cir.const #cir.int<9999> : !s32i
+// CIR: cir.store {{.*}} %[[C9999]], %[[TMP2]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+// CIR: %[[V2:.+]] = cir.load {{.*}} %[[TMP2]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.store {{.*}} %[[V2]], %[[TMP_BARRET]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+// CIR: %[[BARRET_VAL:.+]] = cir.load {{.*}} %[[TMP_BARRET]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.store %[[BARRET_VAL]], %[[RETVAL]] : !s32i, !cir.ptr<!s32i>
+// CIR: %[[RES:.+]] = cir.load %[[RETVAL]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.return %[[RES]] : !s32i
+// CIR: }
+// CIR: %[[FINAL_RES:.+]] = cir.load %[[RETVAL]] : !cir.ptr<!s32i>, !s32i
+// CIR: cir.return %[[FINAL_RES]] : !s32i
+
+// LLVM: define dso_local i32 @nested()
+// LLVM: %[[VAR1:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR2:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR3:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR4:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR5:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR6:.+]] = alloca i32, i64 1
+// LLVM: %[[VAR7:.+]] = alloca i32, i64 1
+// LLVM: br label %[[LBL8:.+]]
+// LLVM: [[LBL8]]:
+// LLVM: store i32 123, ptr %[[VAR7]]
+// LLVM: br label %[[LBL9:.+]]
+// LLVM: [[LBL9]]:
+// LLVM: br label %[[LBL10:.+]]
+// LLVM: [[LBL10]]:
+// LLVM: store i32 987, ptr %[[VAR1]]
+// LLVM: br label %[[LBL11:.+]]
+// LLVM: [[LBL11]]:
+// LLVM: br label %[[LBL12:.+]]
+// LLVM: [[LBL12]]:
+// LLVM: store i32 123, ptr %[[VAR5]]
+// LLVM: %[[V1:.+]] = load i32, ptr %[[VAR5]]
+// LLVM: store i32 %[[V1]], ptr %[[VAR3]]
+// LLVM: br label %[[LBL14:.+]]
+// LLVM: [[LBL14]]:
+// LLVM: br label %[[LBL15:.+]]
+// LLVM: [[LBL15]]:
+// LLVM: store i32 9999, ptr %[[VAR4]]
+// LLVM: br label %[[LBL16:.+]]
+// LLVM: [[LBL16]]:
+// LLVM: %[[V2:.+]] = load i32, ptr %[[VAR4]]
+// LLVM: store i32 %[[V2]], ptr %[[VAR2]]
+// LLVM: br label %[[LBL18:.+]]
+// LLVM: [[LBL18]]:
+// LLVM: %[[V3:.+]] = load i32, ptr %[[VAR2]]
+// LLVM: store i32 %[[V3]], ptr %[[VAR6]]
+// LLVM: %[[RES:.+]] = load i32, ptr %[[VAR6]]
+// LLVM: ret i32 %[[RES]]
+
+// OGCG: define dso_local i32 @nested()
+// OGCG: entry:
+// OGCG: %[[TMP_OUTER:.+]] = alloca i32
+// OGCG: %[[BAR:.+]] = alloca i32
+// OGCG: %[[ASDF:.+]] = alloca i32
+// OGCG: %[[TMP1:.+]] = alloca i32
+// OGCG: %[[TMP2:.+]] = alloca i32
+// OGCG: %[[TMP3:.+]] = alloca i32
+// OGCG: store i32 123, ptr %[[TMP_OUTER]]
+// OGCG: %[[OUTER_VAL:.+]] = load i32, ptr %[[TMP_OUTER]]
+// OGCG: store i32 987, ptr %[[BAR]]
+// OGCG: store i32 123, ptr %[[ASDF]]
+// OGCG: %[[ASDF_VAL:.+]] = load i32, ptr %[[ASDF]]
+// OGCG: store i32 %[[ASDF_VAL]], ptr %[[TMP1]]
+// OGCG: %[[TMP1_VAL:.+]] = load i32, ptr %[[TMP1]]
+// OGCG: store i32 9999, ptr %[[TMP3]]
+// OGCG: %[[TMP3_VAL:.+]] = load i32, ptr %[[TMP3]]
+// OGCG: store i32 %[[TMP3_VAL]], ptr %[[TMP2]]
+// OGCG: %[[RES:.+]] = load i32, ptr %[[TMP2]]
+// OGCG: ret i32 %[[RES]]
+
+void empty() {
+ return ({;;;;});
+}
+
+// CIR: cir.func no_proto dso_local @empty()
+// CIR-NEXT: cir.return
+
+// LLVM: define dso_local void @empty()
+// LLVM: ret void
+// LLVM: }
+
+// OGCG: define dso_local void @empty()
+// OGCG: ret void
+// OGCG: }
|
mmha
force-pushed
the
cir-statement-expressions
branch
3 times, most recently
from
4c0aea8 to
020184b
Compare
andykaylor
reviewed
Aug 14, 2025
| @@ -0,0 +1,166 @@ | |||
| // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir | |||
There was a problem hiding this comment.
Can you add a test that requires a cleanup related to the scope of the statement expression?
In the incubator, I see stmt-expr.c, stmt-expr.cpp, and stmtexpr-init.c. I'm not sure if we have enough support for everything included in these tests, but they have some interesting cases, including a commented out test for a statement containing a label that you've added support for.
There was a problem hiding this comment.
stmtexpr-init.c is not useful because we lack record init list exprs. I added a few C++ tests but I noticed that we don't call destructors for prvalues:
struct with_dtor {
~with_dtor();
};
void cleanup() {
with_dtor{}; // nop with CIR
}So I didn't cover those in the statement expr tests, yet.
There was a problem hiding this comment.
Oh! I would have expected that case to be handled now. We don't even issue a diagnostic saying we missed it. That's not good.
mmha
force-pushed
the
cir-statement-expressions
branch
from
0647eee to
383072c
Compare
andykaylor
reviewed
Aug 15, 2025
clang/lib/CIR/CodeGen/CIRGenExpr.cpp
Outdated
| if (!classDecl->hasTrivialDestructor()) | ||
| referenceTemporaryDtor = | ||
| classDecl->getDefinitionOrSelf()->getDestructor(); | ||
| if (const auto *classDecl = dyn_cast<CXXRecordDecl>( |
There was a problem hiding this comment.
Since this was a cast before, it would have asserted if the return value was null. Are there cases where this returns null?
There was a problem hiding this comment.
Yes, this asserted in C mode where the record was not a C++ class. Note that OGCG is also dyn_casting here.
There was a problem hiding this comment.
test3() contains a MaterializeTemporaryExpr in C: https://godbolt.org/z/b7WY5dfYW
clang/lib/CIR/CodeGen/CIRGenExpr.cpp
Outdated
| if (const auto *classDecl = dyn_cast<CXXRecordDecl>( | ||
| rt->getOriginalDecl()->getDefinitionOrSelf())) { | ||
| if (!classDecl->hasTrivialDestructor()) | ||
| referenceTemporaryDtor = classDecl->getDestructor(); |
There was a problem hiding this comment.
Does this fix the case you mentioned in your comment below about the destructor being called?
There was a problem hiding this comment.
I think I made a copy-paste error here. This line should not have changed. Good catch!
There was a problem hiding this comment.
Why does this differ to traditional codegen, i.e.
llvm-project/clang/lib/CodeGen/CGExpr.cpp
Line 407 in 0e52092
where this is in switch below with slightly different implementation.
There was a problem hiding this comment.
Good question. From what I can tell this was introduced in llvm/clangir#872 and it looks like this slipped through. ReferenceTemporaryDtor should be declared within that switch. I'll make that fix.
mmha
force-pushed
the
cir-statement-expressions
branch
from
03608bb to
87d2918
Compare
This patch adds support for statement expressions. It also changes emitCompoundStmt and emitCompoundStmtWithoutScope to accept an Address that the optional result is written to. This allows the creation of the alloca ahead of the creation of the scope which saves us from hoisting the alloca to its parent scope.
* Failed function definition will not lead to FuncOp being erased * Try to emit statements after a failure in a compound statement
mmha
force-pushed
the
cir-statement-expressions
branch
from
87d2918 to
3485be8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Depends on #153625
This patch adds support for statement expressions. It also changes emitCompoundStmt and emitCompoundStmtWithoutScope to accept an Address that the optional result is written to. This allows the creation of the alloca ahead of the creation of the scope which saves us from hoisting the alloca to its parent scope.