[CIR] Emit init of local variables #130164
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
Local variable initialization was previously being ignored. This change adds support for initialization of scalar variables with constant values and introduces the constant emitter framework.
andykaylor
requested review from
AaronBallman,
AmrDeveloper,
bcardosolopes,
dkolsen-pgi,
erichkeane and
lanza
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesLocal variable initialization was previously being ignored. This change adds support for initialization of scalar variables with constant values and introduces the constant emitter framework. Patch is 48.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130164.diff 14 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index b65797e40d5f9..017ae0c53a984 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -26,6 +26,10 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
CIRBaseBuilderTy(mlir::MLIRContext &mlirContext)
: mlir::OpBuilder(&mlirContext) {}
+ cir::ConstantOp getConstant(mlir::Location loc, mlir::TypedAttr attr) {
+ return create<cir::ConstantOp>(loc, attr.getType(), attr);
+ }
+
cir::ConstantOp getBool(bool state, mlir::Location loc) {
return create<cir::ConstantOp>(loc, getBoolTy(), getCIRBoolAttr(state));
}
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index ece04c225e322..7b3741de29075 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -54,6 +54,20 @@ def CIR_BoolAttr : CIR_Attr<"Bool", "bool", [TypedAttrInterface]> {
}];
}
+//===----------------------------------------------------------------------===//
+// ZeroAttr
+//===----------------------------------------------------------------------===//
+
+def ZeroAttr : CIR_Attr<"Zero", "zero", [TypedAttrInterface]> {
+ let summary = "Attribute to represent zero initialization";
+ let description = [{
+ The ZeroAttr is used to indicate zero initialization on structs.
+ }];
+
+ let parameters = (ins AttributeSelfTypeParameter<"">:$type);
+ let assemblyFormat = [{}];
+}
+
//===----------------------------------------------------------------------===//
// UndefAttr
//===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 6f845b7689e51..d20cd0560a7c1 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -41,16 +41,17 @@ struct MissingFeatures {
static bool supportComdat() { return false; }
// Load/store attributes
- static bool opLoadThreadLocal() { return false; }
+ static bool opLoadStoreThreadLocal() { return false; }
static bool opLoadEmitScalarRangeCheck() { return false; }
static bool opLoadBooleanRepresentation() { return false; }
static bool opLoadStoreTbaa() { return false; }
static bool opLoadStoreMemOrder() { return false; }
static bool opLoadStoreVolatile() { return false; }
static bool opLoadStoreAlignment() { return false; }
+ static bool opLoadStoreAtomic() { return false; }
+ static bool opLoadStoreObjC() { return false; }
// AllocaOp handling
- static bool opAllocaVarDeclContext() { return false; }
static bool opAllocaStaticLocal() { return false; }
static bool opAllocaNonGC() { return false; }
static bool opAllocaImpreciseLifetime() { return false; }
@@ -61,6 +62,7 @@ struct MissingFeatures {
static bool opAllocaReference() { return false; }
static bool opAllocaAnnotations() { return false; }
static bool opAllocaDynAllocSize() { return false; }
+ static bool opAllocaCaptureByInit() { return false; }
// FuncOp handling
static bool opFuncOpenCLKernelMetadata() { return false; }
@@ -76,6 +78,10 @@ struct MissingFeatures {
static bool constructABIArgDirectExtend() { return false; }
static bool opGlobalViewAttr() { return false; }
static bool lowerModeOptLevel() { return false; }
+ static bool opTBAA() { return false; }
+ static bool objCLifetime() { return false; }
+ static bool emitNullabilityCheck() { return false; }
+ static bool astVarDeclInterface() { return false; }
};
} // namespace cir
diff --git a/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
new file mode 100644
index 0000000000000..e29149c2e70f1
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
@@ -0,0 +1,98 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// A helper class for emitting expressions and values as cir::ConstantOp
+// and as initializers for global variables.
+//
+// Note: this is based on clang's LLVM IR codegen in ConstantEmitter.h, reusing
+// this class interface makes it easier move forward with bringing CIR codegen
+// to completion.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_LIB_CIR_CODEGEN_CIRGENCONSTANTEMITTER_H
+#define CLANG_LIB_CIR_CODEGEN_CIRGENCONSTANTEMITTER_H
+
+#include "CIRGenFunction.h"
+#include "CIRGenModule.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang::CIRGen {
+
+class ConstantEmitter {
+public:
+ CIRGenModule &cgm;
+ const CIRGenFunction *cgf;
+
+private:
+ bool abstract = false;
+
+ /// Whether we're in a constant context.
+ bool inConstantContext = false;
+
+public:
+ /// Initialize this emission in the context of the given function.
+ /// Use this if the expression might contain contextual references like
+ /// block addresses or PredefinedExprs.
+ ConstantEmitter(CIRGenFunction &cgf) : cgm(cgf.cgm), cgf(&cgf) {}
+
+ ConstantEmitter(const ConstantEmitter &other) = delete;
+ ConstantEmitter &operator=(const ConstantEmitter &other) = delete;
+
+ // All of the "abstract" emission methods below permit the emission to
+ // be immediately discarded without finalizing anything. Therefore, they
+ // must also promise not to do anything that will, in the future, require
+ // finalization:
+ //
+ // - using the CGF (if present) for anything other than establishing
+ // semantic context; for example, an expression with ignored
+ // side-effects must not be emitted as an abstract expression
+ //
+ // - doing anything that would not be safe to duplicate within an
+ // initializer or to propagate to another context; for example,
+ // side effects, or emitting an initialization that requires a
+ // reference to its current location.
+ mlir::Attribute emitForMemory(mlir::Attribute c, QualType t);
+
+ /// Emit the result of the given expression as an abstract constant,
+ /// asserting that it succeeded. This is only safe to do when the
+ /// expression is known to be a constant expression with either a fairly
+ /// simple type or a known simple form.
+ mlir::Attribute emitAbstract(SourceLocation loc, const APValue &value,
+ QualType t);
+
+ mlir::Attribute tryEmitConstantExpr(const ConstantExpr *CE);
+
+ // These are private helper routines of the constant emitter that
+ // can't actually be private because things are split out into helper
+ // functions and classes.
+
+ mlir::Attribute tryEmitPrivateForVarInit(const VarDecl &d);
+
+ mlir::Attribute tryEmitPrivate(const APValue &value, QualType destType);
+ mlir::Attribute tryEmitPrivateForMemory(const APValue &value, QualType t);
+
+ /// Try to emit the initializer of the given declaration as an abstract
+ /// constant.
+ mlir::Attribute tryEmitAbstractForInitializer(const VarDecl &d);
+
+private:
+ struct AbstractState {
+ bool oldValue;
+ };
+ AbstractState pushAbstract() {
+ AbstractState saved = {abstract};
+ abstract = true;
+ return saved;
+ }
+ mlir::Attribute validateAndPopAbstract(mlir::Attribute C, AbstractState save);
+};
+
+} // namespace clang::CIRGen
+
+#endif // CLANG_LIB_CIR_CODEGEN_CIRGENCONSTANTEMITTER_H
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index 406026b0b9f27..ed42cd6bd2c32 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -10,23 +10,29 @@
//
//===----------------------------------------------------------------------===//
+#include "CIRGenConstantEmitter.h"
#include "CIRGenFunction.h"
+#include "mlir/IR/Location.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/CIR/MissingFeatures.h"
using namespace clang;
using namespace clang::CIRGen;
-void CIRGenFunction::emitAutoVarAlloca(const VarDecl &d) {
+CIRGenFunction::AutoVarEmission
+CIRGenFunction::emitAutoVarAlloca(const VarDecl &d) {
QualType ty = d.getType();
if (ty.getAddressSpace() != LangAS::Default)
cgm.errorNYI(d.getSourceRange(), "emitAutoVarAlloca: address space");
- auto loc = getLoc(d.getSourceRange());
+ mlir::Location loc = getLoc(d.getSourceRange());
- if (d.isEscapingByref())
+ CIRGenFunction::AutoVarEmission emission(d);
+ emission.IsEscapingByRef = d.isEscapingByref();
+ if (emission.IsEscapingByRef)
cgm.errorNYI(d.getSourceRange(),
"emitAutoVarDecl: decl escaping by reference");
@@ -46,21 +52,124 @@ void CIRGenFunction::emitAutoVarAlloca(const VarDecl &d) {
address = createTempAlloca(allocaTy, alignment, loc, d.getName());
declare(address.getPointer(), &d, ty, getLoc(d.getSourceRange()), alignment);
+ emission.Addr = address;
setAddrOfLocalVar(&d, address);
+
+ return emission;
+}
+
+/// Determine whether the given initializer is trivial in the sense
+/// that it requires no code to be generated.
+bool CIRGenFunction::isTrivialInitializer(const Expr *init) {
+ if (!init)
+ return true;
+
+ if (const CXXConstructExpr *construct = dyn_cast<CXXConstructExpr>(init))
+ if (CXXConstructorDecl *constructor = construct->getConstructor())
+ if (constructor->isTrivial() && constructor->isDefaultConstructor() &&
+ !construct->requiresZeroInitialization())
+ return true;
+
+ return false;
}
-void CIRGenFunction::emitAutoVarInit(const clang::VarDecl &d) {
+void CIRGenFunction::emitAutoVarInit(
+ const CIRGenFunction::AutoVarEmission &emission) {
+ const VarDecl &d = *emission.Variable;
+
QualType type = d.getType();
// If this local has an initializer, emit it now.
const Expr *init = d.getInit();
- if (init || !type.isPODType(getContext())) {
- cgm.errorNYI(d.getSourceRange(), "emitAutoVarInit");
+ if (!type.isPODType(getContext())) {
+ cgm.errorNYI(d.getSourceRange(), "emitAutoVarInit: non-POD type");
+ return;
+ }
+
+ const Address addr = emission.Addr;
+
+ // Check whether this is a byref variable that's potentially
+ // captured and moved by its own initializer. If so, we'll need to
+ // emit the initializer first, then copy into the variable.
+ assert(!cir::MissingFeatures::opAllocaCaptureByInit());
+
+ // Note: constexpr already initializes everything correctly.
+ LangOptions::TrivialAutoVarInitKind trivialAutoVarInit =
+ (d.isConstexpr()
+ ? LangOptions::TrivialAutoVarInitKind::Uninitialized
+ : (d.getAttr<UninitializedAttr>()
+ ? LangOptions::TrivialAutoVarInitKind::Uninitialized
+ : getContext().getLangOpts().getTrivialAutoVarInit()));
+
+ auto initializeWhatIsTechnicallyUninitialized = [&](Address addr) {
+ if (trivialAutoVarInit ==
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)
+ return;
+
+ cgm.errorNYI(d.getSourceRange(), "emitAutoVarInit: trivial initialization");
+ };
+
+ if (isTrivialInitializer(init)) {
+ initializeWhatIsTechnicallyUninitialized(addr);
+ return;
+ }
+
+ mlir::Attribute constant;
+ if (emission.IsConstantAggregate ||
+ d.mightBeUsableInConstantExpressions(getContext())) {
+ // FIXME: Differently from LLVM we try not to emit / lower too much
+ // here for CIR since we are interested in seeing the ctor in some
+ // analysis later on. So CIR's implementation of ConstantEmitter will
+ // frequently return an empty Attribute, to signal we want to codegen
+ // some trivial ctor calls and whatnots.
+ constant = ConstantEmitter(*this).tryEmitAbstractForInitializer(d);
+ if (constant && !mlir::isa<cir::ZeroAttr>(constant) &&
+ (trivialAutoVarInit !=
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)) {
+ cgm.errorNYI(d.getSourceRange(), "emitAutoVarInit: constant aggregate");
+ return;
+ }
+ }
+
+ // NOTE(cir): In case we have a constant initializer, we can just emit a
+ // store. But, in CIR, we wish to retain any ctor calls, so if it is a
+ // CXX temporary object creation, we ensure the ctor call is used deferring
+ // its removal/optimization to the CIR lowering.
+ if (!constant || isa<CXXTemporaryObjectExpr>(init)) {
+ initializeWhatIsTechnicallyUninitialized(addr);
+ LValue lv = LValue::makeAddr(addr, type);
+ emitExprAsInit(init, &d, lv);
+ // In case lv has uses it means we indeed initialized something
+ // out of it while trying to build the expression, mark it as such.
+ mlir::Value val = lv.getAddress().getPointer();
+ assert(val && "Should have an address");
+ auto allocaOp = dyn_cast_or_null<cir::AllocaOp>(val.getDefiningOp());
+ assert(allocaOp && "Address should come straight out of the alloca");
+
+ if (!allocaOp.use_empty())
+ allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
+ return;
+ }
+
+ // FIXME(cir): migrate most of this file to use mlir::TypedAttr directly.
+ auto typedConstant = mlir::dyn_cast<mlir::TypedAttr>(constant);
+ assert(typedConstant && "expected typed attribute");
+ if (!emission.IsConstantAggregate) {
+ // For simple scalar/complex initialization, store the value directly.
+ LValue lv = LValue::makeAddr(addr, type);
+ assert(init && "expected initializer");
+ mlir::Location initLoc = getLoc(init->getSourceRange());
+ // lv.setNonGC(true);
+ return emitStoreThroughLValue(
+ RValue::get(builder.getConstant(initLoc, typedConstant)), lv);
}
}
-void CIRGenFunction::emitAutoVarCleanups(const clang::VarDecl &d) {
+void CIRGenFunction::emitAutoVarCleanups(
+ const CIRGenFunction::AutoVarEmission &emission) {
+ const VarDecl &d = *emission.Variable;
+
// Check the type for a cleanup.
if (d.needsDestruction(getContext()))
cgm.errorNYI(d.getSourceRange(), "emitAutoVarCleanups: type cleanup");
@@ -76,9 +185,9 @@ void CIRGenFunction::emitAutoVarCleanups(const clang::VarDecl &d) {
/// register, or no storage class specifier. These turn into simple stack
/// objects, globals depending on target.
void CIRGenFunction::emitAutoVarDecl(const VarDecl &d) {
- emitAutoVarAlloca(d);
- emitAutoVarInit(d);
- emitAutoVarCleanups(d);
+ CIRGenFunction::AutoVarEmission emission = emitAutoVarAlloca(d);
+ emitAutoVarInit(emission);
+ emitAutoVarCleanups(emission);
}
void CIRGenFunction::emitVarDecl(const VarDecl &d) {
@@ -94,10 +203,59 @@ void CIRGenFunction::emitVarDecl(const VarDecl &d) {
assert(d.hasLocalStorage());
- assert(!cir::MissingFeatures::opAllocaVarDeclContext());
+ CIRGenFunction::VarDeclContext varDeclCtx{*this, &d};
return emitAutoVarDecl(d);
}
+void CIRGenFunction::emitScalarInit(const Expr *init, mlir::Location loc,
+ LValue lvalue, bool capturedByInit) {
+ Qualifiers::ObjCLifetime lifetime = Qualifiers::ObjCLifetime::OCL_None;
+ assert(!cir::MissingFeatures::objCLifetime());
+
+ if (!lifetime) {
+ SourceLocRAIIObject locRAII{*this, loc};
+ mlir::Value value = emitScalarExpr(init);
+ if (capturedByInit) {
+ cgm.errorNYI(init->getSourceRange(), "emitScalarInit: captured by init");
+ return;
+ }
+ assert(!cir::MissingFeatures::emitNullabilityCheck());
+ emitStoreThroughLValue(RValue::get(value), lvalue, true);
+ return;
+ }
+
+ cgm.errorNYI(loc, "emitScalarInit: ObjCLifetime");
+}
+
+void CIRGenFunction::emitExprAsInit(const Expr *init, const ValueDecl *d,
+ LValue lvalue, bool capturedByInit) {
+ SourceLocRAIIObject loc{*this, getLoc(init->getSourceRange())};
+ if (capturedByInit) {
+ cgm.errorNYI(init->getSourceRange(), "emitExprAsInit: captured by init");
+ return;
+ }
+
+ QualType type = d->getType();
+
+ if (type->isReferenceType()) {
+ cgm.errorNYI(init->getSourceRange(), "emitExprAsInit: reference type");
+ return;
+ }
+ switch (CIRGenFunction::getEvaluationKind(type)) {
+ case cir::TEK_Scalar:
+ emitScalarInit(init, getLoc(d->getSourceRange()), lvalue);
+ return;
+ case cir::TEK_Complex: {
+ cgm.errorNYI(init->getSourceRange(), "emitExprAsInit: complex type");
+ return;
+ }
+ case cir::TEK_Aggregate:
+ cgm.errorNYI(init->getSourceRange(), "emitExprAsInit: aggregate type");
+ return;
+ }
+ llvm_unreachable("bad evaluation kind");
+}
+
void CIRGenFunction::emitDecl(const Decl &d) {
switch (d.getKind()) {
case Decl::Var: {
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index ccc3e20875263..07fb4cf8f1513 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -25,9 +25,77 @@ using namespace clang;
using namespace clang::CIRGen;
using namespace cir;
+void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst,
+ bool isInit) {
+ if (!dst.isSimple()) {
+ cgm.errorNYI(dst.getPointer().getLoc(),
+ "emitStoreThroughLValue: non-simple lvalue");
+ return;
+ }
+
+ assert(!cir::MissingFeatures::opLoadStoreObjC());
+
+ assert(src.isScalar() && "Can't emit an aggregate store with this method");
+ emitStoreOfScalar(src.getScalarVal(), dst, isInit);
+}
+
+void CIRGenFunction::emitStoreOfScalar(mlir::Value value, Address addr,
+ bool isVolatile, QualType ty,
+ bool isInit, bool isNontemporal) {
+ assert(!cir::MissingFeatures::opLoadStoreThreadLocal());
+
+ if (ty->getAs<clang::VectorType>()) {
+ cgm.errorNYI(addr.getPointer().getLoc(), "emitStoreOfScalar vector type");
+ return;
+ }
+
+ value = emitToMemory(value, ty);
+
+ assert(!cir::MissingFeatures::opLoadStoreAtomic());
+
+ // Update the alloca with more info on initialization.
+ assert(addr.getPointer() && "expected pointer to exist");
+ auto srcAlloca =
+ dyn_cast_or_null<cir::AllocaOp>(addr.getPointer().getDefiningOp());
+ if (currVarDecl && srcAlloca) {
+ const VarDecl *vd = currVarDecl;
+ assert(vd && "VarDecl expected");
+ if (vd->hasInit())
+ srcAlloca.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
+ }
+
+ assert(currSrcLoc && "must pass in source location");
+ builder.createStore(*currSrcLoc, value, addr.getPointer() /*, isVolatile*/);
+
+ if (isNontemporal) {
+ cgm.errorNYI(addr.getPointer().getLoc(), "emitStoreOfScalar nontemporal");
+ return;
+ }
+
+ assert(!cir::MissingFeatures::opTBAA());
+}
+
+mlir::Value CIRGenFunction::emitToMemory(mlir::Value value, QualType ty) {
+ // Bool has a different representation in memory than in registers,
+ // but in ClangIR, it is simply represented as a cir.bool value.
+ // This function is here as a placeholder for possible future changes.
+ return value;
+}
+
+void CIRGenFunction::emitStoreOfScalar(mlir::Value value, LValue lvalue,
+ bool isInit) {
+ if (lvalue.getType()->isConstantMatrixType()) {
+ assert(0 && "NYI: emitStoreOfScalar constant matrix type");
+ return;
+ }
+
+ emitStoreOfScalar(value, lvalue.getAddress(), lvalue.isVolatile(),
+ lvalue.getType(), isInit, /*isNontemporal=*/false);
+}
+
mlir::Value CIRGenFunction::emitLoadOfScalar(LValue lvalue,
SourceLocation loc) {
- assert(!cir::MissingFeatures::opLoadThreadLocal());
+ assert(!cir::MissingFeatures::opLoadStoreThreadLocal());
assert(!cir::MissingFeatures::opLoadEmitScalarRangeCheck());
assert(!cir::MissingFeatures::opLoadBooleanRepresentation());
@@ -115,7 +183,7 @@ mlir::Value CIRGenFunction::emitAlloca(StringRef name, mlir::Type ty,
builder.restoreInsertionPoint(builder.getBestAllocaInsertPoint(entryBlock));
addr = builder.createAlloca(loc, /*addr type*/ localVarPtrTy,
/*var type*/ ty, name, alignIntAttr);
- assert(!cir::MissingFeatures::opAllocaVarDeclContext());
+ assert(!cir::MissingFeatures::a...
[truncated]
|
erichkeane
reviewed
Mar 6, 2025
Collaborator
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
1 comment, but I don't feel comfortable approving this with some of the work, so I'll count on the CIR reviewers (David, Henrick, Bruno, Nathan, etc) to do the approval.
| QualType destType) { | ||
| AbstractState state = pushAbstract(); | ||
| mlir::Attribute c = tryEmitPrivate(value, destType); | ||
| c = validateAndPopAbstract(c, state); |
Collaborator
There was a problem hiding this comment.
BOTH cases we're doing pushAbstract and validateAndPopAbstract. It seems to me that this should be a RAII container for push/pop, and have validateAttribute function instead.
Contributor
Author
There was a problem hiding this comment.
I should probably have looked at this a bit closer. I think the idea is that validateAndPopAbstract might need to do some kind of clean up and could possibly fail, but that isn't happening, even in the LLVM IR codegen that this was modeled after. In the original codegen, it asserts that the placeholder list size hasn't changed, but it doesn't do anything with the constant. I don't see any problem with making this an RAII.
xlauko
reviewed
Mar 6, 2025
|
|
||
| /// Determine whether the given initializer is trivial in the sense | ||
| /// that it requires no code to be generated. | ||
| bool CIRGenFunction::isTrivialInitializer(const Expr *init) { |
Contributor
There was a problem hiding this comment.
Candidate to be factored out to common codegen.
Contributor
Author
There was a problem hiding this comment.
I'm going to defer this for now, but I agree this could be moved. I just think we need a more solid plan for where to put such things. @dkolsen-pgi was looking into this, I believe.
| void CIRGenFunction::emitAutoVarInit( | ||
| const CIRGenFunction::AutoVarEmission &emission) { | ||
| const VarDecl &d = *emission.Variable; | ||
|
|
Contributor
There was a problem hiding this comment.
Corresponding CodeGenFunction::EmitAutoVarInit has here:
// If this was emitted as a global constant, we're done.
if (emission.wasEmittedAsGlobal()) return;
is there any reason why this is not here for CIR?
Contributor
Author
There was a problem hiding this comment.
No. That should be here.
clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Outdated
|
|
||
| void CIRGenFunction::emitScalarInit(const Expr *init, mlir::Location loc, | ||
| LValue lvalue, bool capturedByInit) { | ||
| Qualifiers::ObjCLifetime lifetime = Qualifiers::ObjCLifetime::OCL_None; |
Contributor
There was a problem hiding this comment.
Why not:
Qualifiers::ObjCLifetime lifetime = lvalue.getObjCLifetime();
as in main codegen?
Contributor
Author
There was a problem hiding this comment.
That's not implemented yet. CIR has its own LValue class, and getObjCLifetime() hasn't been added there.
Contributor
Author
|
Apparently the code formatting checker won't even let me mention "undef" in a comment. |
bcardosolopes
approved these changes
Mar 7, 2025
Member
bcardosolopes
left a comment
bcardosolopes
left a comment
There was a problem hiding this comment.
LGTM after addressing other comments
erichkeane
approved these changes
Mar 7, 2025
erichkeane
reviewed
Jul 10, 2025
|
|
||
| public: | ||
| VarDeclContext(CIRGenFunction &p, const VarDecl *value) : p(p) { | ||
| if (p.currVarDecl) |
Collaborator
There was a problem hiding this comment.
SO I was going through debugging and found that this is somewhat odd/looks strange to me. Is there a good reason that we have this guarded to set oldVal, but unconditionally restore it? I realize this ends up 'working' because currVarDecl is a pointer (and oldVal is initialized to a nullptr), thus this ends up working, but this condition seems REALLY strange. It is at best confusing/unnecessary, at worst, a bug in restore.
Contributor
Author
There was a problem hiding this comment.
You're right. I can see no reason this condition needs to be here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
Local variable initialization was previously being ignored. This change adds support for initialization of scalar variables with constant values and introduces the constant emitter framework.