Skip to content

[CIR][NFC] Upstream LValueBaseInfo handling #134928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2025
Merged

[CIR][NFC] Upstream LValueBaseInfo handling #134928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/include/clang/CIR/MissingFeatures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ struct MissingFeatures {
static bool cgFPOptionsRAII() { return false; }
static bool metaDataNode() { return false; }
static bool fastMathFlags() { return false; }
static bool lvalueBaseInfo() { return false; }
static bool alignCXXRecordDecl() { return false; }
static bool setNonGC() { return false; }
static bool incrementProfileCounter() { return false; }
static bool insertBuiltinUnpredictable() { return false; }
static bool objCGC() { return false; }

// Missing types
static bool dataMemberType() { return false; }
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void CIRGenFunction::emitAutoVarInit(
// its removal/optimization to the CIR lowering.
if (!constant || isa<CXXTemporaryObjectExpr>(init)) {
initializeWhatIsTechnicallyUninitialized(addr);
LValue lv = LValue::makeAddr(addr, type);
LValue lv = makeAddrLValue(addr, type, AlignmentSource::Decl);
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.
Expand All @@ -165,7 +165,7 @@ void CIRGenFunction::emitAutoVarInit(
assert(typedConstant && "expected typed attribute");
if (!emission.IsConstantAggregate) {
// For simple scalar/complex initialization, store the value directly.
LValue lv = LValue::makeAddr(addr, type);
LValue lv = makeAddrLValue(addr, type);
assert(init && "expected initializer");
mlir::Location initLoc = getLoc(init->getSourceRange());
// lv.setNonGC(true);
Expand Down
14 changes: 8 additions & 6 deletions clang/lib/CIR/CodeGen/CIRGenExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ using namespace cir;

/// Given an expression of pointer type, try to
/// derive a more accurate bound on the alignment of the pointer.
Address CIRGenFunction::emitPointerWithAlignment(const Expr *expr) {
Address CIRGenFunction::emitPointerWithAlignment(const Expr *expr,
LValueBaseInfo *baseInfo) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a new warning from the previous PR related to unused variable, should we quickly handle it here?

if (auto const *ece = dyn_cast<ExplicitCastExpr>(ce)) { <----- ece
   cgm.errorNYI(expr->getSourceRange(),
                   "emitPointerWithAlignment: explicit cast");
   return Address::invalid();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It's best to keep changes like that separate to avoid mingling unrelated changes in the commit history. We wouldn't want a warning to be reintroduced if the commit for this PR were reverted, for example. Thanks for brining this to my attention though, I'll take care of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that make sense

// We allow this with ObjC object pointers because of fragile ABIs.
assert(expr->getType()->isPointerType() ||
expr->getType()->isObjCObjectPointerType());
Expand Down Expand Up @@ -164,7 +165,8 @@ Address CIRGenFunction::emitPointerWithAlignment(const Expr *expr) {

// Otherwise, use the alignment of the type.
return makeNaturalAddressForPointer(
emitScalarExpr(expr), expr->getType()->getPointeeType(), CharUnits());
emitScalarExpr(expr), expr->getType()->getPointeeType(), CharUnits(),
/*forPointeeType=*/true, baseInfo);
}

void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst,
Expand Down Expand Up @@ -300,7 +302,7 @@ LValue CIRGenFunction::emitDeclRefLValue(const DeclRefExpr *e) {
cgm.errorNYI(vd->getSourceRange(), "emitDeclRefLValue: static local");
}

return LValue::makeAddr(addr, ty);
return makeAddrLValue(addr, ty, AlignmentSource::Type);
}

cgm.errorNYI(e->getSourceRange(), "emitDeclRefLValue: unhandled decl type");
Expand Down Expand Up @@ -338,9 +340,9 @@ LValue CIRGenFunction::emitUnaryOpLValue(const UnaryOperator *e) {
QualType t = e->getSubExpr()->getType()->getPointeeType();
assert(!t.isNull() && "CodeGenFunction::EmitUnaryOpLValue: Illegal type");

assert(!cir::MissingFeatures::lvalueBaseInfo());
assert(!cir::MissingFeatures::opTBAA());
Address addr = emitPointerWithAlignment(e->getSubExpr());
LValueBaseInfo baseInfo;
Address addr = emitPointerWithAlignment(e->getSubExpr(), &baseInfo);

// Tag 'load' with deref attribute.
// FIXME: This misses some derefence cases and has problematic interactions
Expand All @@ -350,7 +352,7 @@ LValue CIRGenFunction::emitUnaryOpLValue(const UnaryOperator *e) {
loadOp.setIsDerefAttr(mlir::UnitAttr::get(&getMLIRContext()));
}

LValue lv = LValue::makeAddr(addr, t);
LValue lv = makeAddrLValue(addr, t, baseInfo);
assert(!cir::MissingFeatures::addressSpace());
assert(!cir::MissingFeatures::setNonGC());
return lv;
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void AggExprEmitter::emitArrayInit(Address destPtr, cir::ArrayType arrayTy,
}

const Address address = Address(element, cirElementType, elementAlign);
const LValue elementLV = LValue::makeAddr(address, elementType);
const LValue elementLV = cgf.makeAddrLValue(address, elementType);
emitInitializationToLValue(args[i], elementLV);
}

Expand All @@ -157,7 +157,7 @@ void AggExprEmitter::emitArrayInit(Address destPtr, cir::ArrayType arrayTy,
const Address tmpAddr = cgf.createTempAlloca(
cirElementPtrType, cgf.getPointerAlign(), loc, "arrayinit.temp",
/*insertIntoFnEntryBlock=*/false);
LValue tmpLV = LValue::makeAddr(tmpAddr, elementPtrType);
LValue tmpLV = cgf.makeAddrLValue(tmpAddr, elementPtrType);
cgf.emitStoreThroughLValue(RValue::get(element), tmpLV);

// TODO(CIR): Replace this part later with cir::DoWhileOp
Expand All @@ -166,7 +166,7 @@ void AggExprEmitter::emitArrayInit(Address destPtr, cir::ArrayType arrayTy,
builder.createLoad(loc, tmpAddr.getPointer());

// Emit the actual filler expression.
const LValue elementLV = LValue::makeAddr(
const LValue elementLV = cgf.makeAddrLValue(
Address(currentElement, cirElementType, elementAlign), elementType);

if (arrayFiller)
Expand Down
18 changes: 15 additions & 3 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,23 @@ class CIRGenFunction : public CIRGenTypeCache {
/// been signed, and the returned Address will have the pointer authentication
/// information needed to authenticate the signed pointer.
Address makeNaturalAddressForPointer(mlir::Value ptr, QualType t,
CharUnits alignment) {
CharUnits alignment,
bool forPointeeType = false,
LValueBaseInfo *baseInfo = nullptr) {
if (alignment.isZero())
alignment = cgm.getNaturalTypeAlignment(t);
alignment = cgm.getNaturalTypeAlignment(t, baseInfo);
return Address(ptr, convertTypeForMem(t), alignment);
}

LValue makeAddrLValue(Address addr, QualType ty,
AlignmentSource source = AlignmentSource::Type) {
return makeAddrLValue(addr, ty, LValueBaseInfo(source));
}

LValue makeAddrLValue(Address addr, QualType ty, LValueBaseInfo baseInfo) {
return LValue::makeAddr(addr, ty, baseInfo);
}

cir::FuncOp generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
cir::FuncType funcType);

Expand Down Expand Up @@ -523,7 +534,8 @@ class CIRGenFunction : public CIRGenTypeCache {
/// into the address of a local variable. In such a case, it's quite
/// reasonable to just ignore the returned alignment when it isn't from an
/// explicit source.
Address emitPointerWithAlignment(const clang::Expr *expr);
Address emitPointerWithAlignment(const clang::Expr *expr,
LValueBaseInfo *baseInfo);

mlir::LogicalResult emitReturnStmt(const clang::ReturnStmt &s);

Expand Down
18 changes: 11 additions & 7 deletions clang/lib/CIR/CodeGen/CIRGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,20 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &mlirContext,
builder.getStringAttr(getTriple().str()));
}

CharUnits CIRGenModule::getNaturalTypeAlignment(QualType t) {
CharUnits CIRGenModule::getNaturalTypeAlignment(QualType t,
LValueBaseInfo *baseInfo) {
assert(!cir::MissingFeatures::opTBAA());

// FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown. But
// that doesn't return the information we need to compute BaseInfo.
// FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown, but
// that doesn't return the information we need to compute baseInfo.

// Honor alignment typedef attributes even on incomplete types.
// We also honor them straight for C++ class types, even as pointees;
// there's an expressivity gap here.
if (const auto *tt = t->getAs<TypedefType>()) {
if (unsigned align = tt->getDecl()->getMaxAlignment()) {
assert(!cir::MissingFeatures::lvalueBaseInfo());
if (baseInfo)
*baseInfo = LValueBaseInfo(AlignmentSource::AttributedType);
return astContext.toCharUnitsFromBits(align);
}
}
Expand All @@ -99,13 +101,15 @@ CharUnits CIRGenModule::getNaturalTypeAlignment(QualType t) {
// ASTContext::getTypeAlignIfKnown, but nothing uses the alignment if the
// type is incomplete, so it's impossible to test. We could try to reuse
// getTypeAlignIfKnown, but that doesn't return the information we need
// to set BaseInfo. So just ignore the possibility that the alignment is
// to set baseInfo. So just ignore the possibility that the alignment is
// greater than one.
assert(!cir::MissingFeatures::lvalueBaseInfo());
if (baseInfo)
*baseInfo = LValueBaseInfo(AlignmentSource::Type);
return CharUnits::One();
}

assert(!cir::MissingFeatures::lvalueBaseInfo());
if (baseInfo)
*baseInfo = LValueBaseInfo(AlignmentSource::Type);

CharUnits alignment;
if (t.getQualifiers().hasUnaligned()) {
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "CIRGenBuilder.h"
#include "CIRGenTypeCache.h"
#include "CIRGenTypes.h"
#include "CIRGenValue.h"

#include "clang/AST/CharUnits.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
Expand Down Expand Up @@ -91,7 +92,8 @@ class CIRGenModule : public CIRGenTypeCache {

/// FIXME: this could likely be a common helper and not necessarily related
/// with codegen.
clang::CharUnits getNaturalTypeAlignment(clang::QualType t);
clang::CharUnits getNaturalTypeAlignment(clang::QualType t,
LValueBaseInfo *baseInfo);

void emitTopLevelDecl(clang::Decl *decl);

Expand Down
53 changes: 46 additions & 7 deletions clang/lib/CIR/CodeGen/CIRGenValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ enum class AlignmentSource {
Type
};

/// Given that the base address has the given alignment source, what's
/// our confidence in the alignment of the field?
static inline AlignmentSource getFieldAlignmentSource(AlignmentSource source) {
// For now, we don't distinguish fields of opaque pointers from
// top-level declarations, but maybe we should.
return AlignmentSource::Decl;
}

class LValueBaseInfo {
AlignmentSource alignSource;

public:
explicit LValueBaseInfo(AlignmentSource source = AlignmentSource::Type)
: alignSource(source) {}
AlignmentSource getAlignmentSource() const { return alignSource; }
void setAlignmentSource(AlignmentSource source) { alignSource = source; }

void mergeForCast(const LValueBaseInfo &info) {
setAlignmentSource(info.getAlignmentSource());
}
};

class LValue {
enum {
Simple, // This is a normal l-value, use getAddress().
Expand All @@ -87,13 +109,26 @@ class LValue {
clang::QualType type;
clang::Qualifiers quals;

// The alignment to use when accessing this lvalue. (For vector elements,
// this is the alignment of the whole vector)
unsigned alignment;
mlir::Value v;
mlir::Type elementType;
LValueBaseInfo baseInfo;

void initialize(clang::QualType type, clang::Qualifiers quals) {
assert(!cir::MissingFeatures::lvalueBaseInfo());
void initialize(clang::QualType type, clang::Qualifiers quals,
clang::CharUnits alignment, LValueBaseInfo baseInfo) {
assert((!alignment.isZero() || type->isIncompleteType()) &&
"initializing l-value with zero alignment!");
this->type = type;
this->quals = quals;
const unsigned maxAlign = 1U << 31;
this->alignment = alignment.getQuantity() <= maxAlign
? alignment.getQuantity()
: maxAlign;
assert(this->alignment == alignment.getQuantity() &&
"Alignment exceeds allowed max!");
this->baseInfo = baseInfo;
}

public:
Expand All @@ -108,23 +143,27 @@ class LValue {
mlir::Value getPointer() const { return v; }

clang::CharUnits getAlignment() const {
// TODO: Handle alignment
return clang::CharUnits::One();
return clang::CharUnits::fromQuantity(alignment);
}
void setAlignment(clang::CharUnits a) { alignment = a.getQuantity(); }

Address getAddress() const {
return Address(getPointer(), elementType, getAlignment());
}

const clang::Qualifiers &getQuals() const { return quals; }

static LValue makeAddr(Address address, clang::QualType t) {
static LValue makeAddr(Address address, clang::QualType t,
LValueBaseInfo baseInfo) {
// Classic codegen sets the objc gc qualifier here. That requires an
// ASTContext, which is passed in from CIRGenFunction::makeAddrLValue.
assert(!cir::MissingFeatures::objCGC());

LValue r;
r.lvType = Simple;
r.v = address.getPointer();
r.elementType = address.getElementType();
r.initialize(t, t.getQualifiers());
assert(!cir::MissingFeatures::lvalueBaseInfo());
r.initialize(t, t.getQualifiers(), address.getAlignment(), baseInfo);
return r;
}
};
Expand Down