[CIR] Upstream support for accessing structure members #136383
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
bcardosolopes,
dkolsen-pgi,
erichkeane,
lanza and
xlauko
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis adds ClangIR support for accessing structure members. Access to union members is deferred to a later change. Patch is 32.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/136383.diff 18 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index d2a241964f34f..5eac4fedaec75 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -191,6 +191,12 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
return create<cir::StoreOp>(loc, val, dst);
}
+ cir::GetMemberOp createGetMember(mlir::Location loc, mlir::Type resultTy,
+ mlir::Value base, llvm::StringRef name,
+ unsigned index) {
+ return create<cir::GetMemberOp>(loc, resultTy, base, name, index);
+ }
+
mlir::Value createDummyValue(mlir::Location loc, mlir::Type type,
clang::CharUnits alignment) {
auto addr = createAlloca(loc, getPointerTo(type), type, {},
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 5ba4b33dc1a12..80b875b2c94ce 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1310,6 +1310,70 @@ def GetGlobalOp : CIR_Op<"get_global",
let hasVerifier = 0;
}
+//===----------------------------------------------------------------------===//
+// GetMemberOp
+//===----------------------------------------------------------------------===//
+
+def GetMemberOp : CIR_Op<"get_member"> {
+ let summary = "Get the address of a member of a record";
+ let description = [{
+ The `cir.get_member` operation gets the address of a particular named
+ member from the input record.
+
+ It expects a pointer to the base record as well as the name of the member
+ and its field index.
+
+ Example:
+ ```mlir
+ // Suppose we have a record with multiple members.
+ !s32i = !cir.int<s, 32>
+ !s8i = !cir.int<s, 32>
+ !ty_B = !cir.record<"struct.B" {!s32i, !s8i}>
+
+ // Get the address of the member at index 1.
+ %1 = cir.get_member %0[1] {name = "i"} : (!cir.ptr<!ty_B>) -> !cir.ptr<!s8i>
+ ```
+ }];
+
+ let arguments = (ins
+ Arg<CIR_PointerType, "the address to load from", [MemRead]>:$addr,
+ StrAttr:$name,
+ IndexAttr:$index_attr);
+
+ let results = (outs Res<CIR_PointerType, "">:$result);
+
+ let assemblyFormat = [{
+ $addr `[` $index_attr `]` attr-dict
+ `:` qualified(type($addr)) `->` qualified(type($result))
+ }];
+
+ let builders = [
+ OpBuilder<(ins "mlir::Type":$type,
+ "mlir::Value":$value,
+ "llvm::StringRef":$name,
+ "unsigned":$index),
+ [{
+ mlir::APInt fieldIdx(64, index);
+ build($_builder, $_state, type, value, name, fieldIdx);
+ }]>
+ ];
+
+ let extraClassDeclaration = [{
+ /// Return the index of the record member being accessed.
+ uint64_t getIndex() { return getIndexAttr().getZExtValue(); }
+
+ /// Return the record type pointed by the base pointer.
+ cir::PointerType getAddrTy() { return getAddr().getType(); }
+
+ /// Return the result type.
+ cir::PointerType getResultTy() {
+ return mlir::cast<cir::PointerType>(getResult().getType());
+ }
+ }];
+
+ let hasVerifier = 1;
+}
+
//===----------------------------------------------------------------------===//
// FuncOp
//===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
index b028bc7db4e59..27ee5389723fa 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -503,6 +503,9 @@ def CIR_RecordType : CIR_Type<"Record", "record",
void complete(llvm::ArrayRef<mlir::Type> members, bool packed,
bool isPadded);
+ uint64_t getElementOffset(const mlir::DataLayout &dataLayout,
+ unsigned idx) const;
+
private:
unsigned computeStructSize(const mlir::DataLayout &dataLayout) const;
uint64_t computeStructAlignment(const mlir::DataLayout &dataLayout) const;
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 0105d1bdaf3fd..6bfc1199aea55 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -157,6 +157,8 @@ struct MissingFeatures {
static bool emitCheckedInBoundsGEP() { return false; }
static bool preservedAccessIndexRegion() { return false; }
static bool bitfields() { return false; }
+ static bool typeChecks() { return false; }
+ static bool lambdaFieldToName() { return false; }
// Missing types
static bool dataMemberType() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index aca26526b79f2..0a518c0fd935d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -27,6 +27,38 @@ using namespace clang;
using namespace clang::CIRGen;
using namespace cir;
+/// Get the address of a zero-sized field within a record. The resulting address
+/// doesn't necessarily have the right type.
+Address CIRGenFunction::emitAddrOfFieldStorage(Address base,
+ const FieldDecl *field,
+ llvm::StringRef fieldName,
+ unsigned fieldIndex) {
+ if (field->isZeroSize(getContext())) {
+ cgm.errorNYI(field->getSourceRange(),
+ "emitAddrOfFieldStorage: zero-sized field");
+ return Address::invalid();
+ }
+
+ mlir::Location loc = getLoc(field->getLocation());
+
+ mlir::Type fieldType = convertType(field->getType());
+ auto fieldPtr = cir::PointerType::get(builder.getContext(), fieldType);
+ // For most cases fieldName is the same as field->getName() but for lambdas,
+ // which do not currently carry the name, so it can be passed down from the
+ // CaptureStmt.
+ cir::GetMemberOp memberAddr = builder.createGetMember(
+ loc, fieldPtr, base.getPointer(), fieldName, fieldIndex);
+
+ // Retrieve layout information, compute alignment and return the final
+ // address.
+ const RecordDecl *rec = field->getParent();
+ const CIRGenRecordLayout &layout = cgm.getTypes().getCIRGenRecordLayout(rec);
+ unsigned idx = layout.getCIRFieldNo(field);
+ CharUnits offset = CharUnits::fromQuantity(
+ layout.getCIRType().getElementOffset(cgm.getDataLayout().layout, idx));
+ return Address(memberAddr, base.getAlignment().alignmentAtOffset(offset));
+}
+
/// 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,
@@ -264,6 +296,66 @@ mlir::Value CIRGenFunction::emitStoreThroughBitfieldLValue(RValue src,
return {};
}
+LValue CIRGenFunction::emitLValueForField(LValue base, const FieldDecl *field) {
+ LValueBaseInfo baseInfo = base.getBaseInfo();
+
+ if (field->isBitField()) {
+ cgm.errorNYI(field->getSourceRange(), "emitLValueForField: bitfield");
+ return LValue();
+ }
+
+ QualType fieldType = field->getType();
+ const RecordDecl *rec = field->getParent();
+ AlignmentSource baseAlignSource = baseInfo.getAlignmentSource();
+ LValueBaseInfo fieldBaseInfo(getFieldAlignmentSource(baseAlignSource));
+ assert(!cir::MissingFeatures::opTBAA());
+
+ Address addr = base.getAddress();
+ if (auto *classDef = dyn_cast<CXXRecordDecl>(rec)) {
+ cgm.errorNYI(field->getSourceRange(), "emitLValueForField: C++ class");
+ return LValue();
+ }
+
+ unsigned recordCVR = base.getVRQualifiers();
+ if (rec->isUnion()) {
+ cgm.errorNYI(field->getSourceRange(), "emitLValueForField: union");
+ return LValue();
+ }
+
+ assert(!cir::MissingFeatures::preservedAccessIndexRegion());
+ llvm::StringRef fieldName = field->getName();
+ const CIRGenRecordLayout &layout =
+ cgm.getTypes().getCIRGenRecordLayout(field->getParent());
+ unsigned fieldIndex = layout.getCIRFieldNo(field);
+
+ assert(!cir::MissingFeatures::lambdaFieldToName());
+
+ addr = emitAddrOfFieldStorage(addr, field, fieldName, fieldIndex);
+
+ // If this is a reference field, load the reference right now.
+ if (fieldType->isReferenceType()) {
+ cgm.errorNYI(field->getSourceRange(), "emitLValueForField: reference type");
+ return LValue();
+ }
+
+ if (field->hasAttr<AnnotateAttr>()) {
+ cgm.errorNYI(field->getSourceRange(), "emitLValueForField: AnnotateAttr");
+ return LValue();
+ }
+
+ LValue lv = makeAddrLValue(addr, fieldType, fieldBaseInfo);
+ lv.getQuals().addCVRQualifiers(recordCVR);
+
+ // __weak attribute on a field is ignored.
+ if (lv.getQuals().getObjCGCAttr() == Qualifiers::Weak) {
+ cgm.errorNYI(field->getSourceRange(),
+ "emitLValueForField: __weak attribute");
+ return LValue();
+ }
+
+ return lv;
+}
+
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.
@@ -608,6 +700,48 @@ CIRGenFunction::emitArraySubscriptExpr(const clang::ArraySubscriptExpr *e) {
return lv;
}
+LValue CIRGenFunction::emitMemberExpr(const MemberExpr *e) {
+ if (auto *vd = dyn_cast<VarDecl>(e->getMemberDecl())) {
+ cgm.errorNYI(e->getSourceRange(), "emitMemberExpr: VarDecl");
+ return LValue();
+ }
+
+ Expr *baseExpr = e->getBase();
+ // If this is s.x, emit s as an lvalue. If it is s->x, emit s as a scalar.
+ LValue baseLV;
+ if (e->isArrow()) {
+ LValueBaseInfo baseInfo;
+ assert(!cir::MissingFeatures::opTBAA());
+ Address addr = emitPointerWithAlignment(baseExpr, &baseInfo);
+ QualType ptrTy = baseExpr->getType()->getPointeeType();
+ assert(!cir::MissingFeatures::typeChecks());
+ baseLV = makeAddrLValue(addr, ptrTy, baseInfo);
+ } else {
+ assert(!cir::MissingFeatures::typeChecks());
+ baseLV = emitLValue(baseExpr);
+ }
+
+ const NamedDecl *nd = e->getMemberDecl();
+ if (auto *field = dyn_cast<FieldDecl>(nd)) {
+ LValue lv = emitLValueForField(baseLV, field);
+ assert(!cir::MissingFeatures::setObjCGCLValueClass());
+ if (getLangOpts().OpenMP) {
+ // If the member was explicitly marked as nontemporal, mark it as
+ // nontemporal. If the base lvalue is marked as nontemporal, mark access
+ // to children as nontemporal too.
+ cgm.errorNYI(e->getSourceRange(), "emitMemberExpr: OpenMP");
+ }
+ return lv;
+ }
+
+ if (const auto *fd = dyn_cast<FunctionDecl>(nd)) {
+ cgm.errorNYI(e->getSourceRange(), "emitMemberExpr: FunctionDecl");
+ return LValue();
+ }
+
+ llvm_unreachable("Unhandled member declaration!");
+}
+
LValue CIRGenFunction::emitBinaryOperatorLValue(const BinaryOperator *e) {
// Comma expressions just emit their LHS then their RHS as an l-value.
if (e->getOpcode() == BO_Comma) {
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 1bef1b976a4b5..05b337b52cbe9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -171,6 +171,8 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
return emitLoadOfLValue(e);
}
+ mlir::Value VisitMemberExpr(MemberExpr *e);
+
mlir::Value VisitExplicitCastExpr(ExplicitCastExpr *e) {
return VisitCastExpr(e);
}
@@ -1529,6 +1531,19 @@ mlir::Value ScalarExprEmitter::VisitCallExpr(const CallExpr *e) {
return v;
}
+mlir::Value ScalarExprEmitter::VisitMemberExpr(MemberExpr *e) {
+ // TODO(cir): The classic codegen calls tryEmitAsConstant() here. Folding
+ // constants sound like work for MLIR optimizers, but we'll keep an assertion
+ // for now.
+ assert(!cir::MissingFeatures::tryEmitAsConstant());
+ Expr::EvalResult result;
+ if (e->EvaluateAsInt(result, cgf.getContext(), Expr::SE_AllowSideEffects)) {
+ cgf.cgm.errorNYI(e->getSourceRange(), "Constant interger member expr");
+ // Fall through to emit this as a non-constant access.
+ }
+ return emitLoadOfLValue(e);
+}
+
mlir::Value CIRGenFunction::emitScalarConversion(mlir::Value src,
QualType srcTy, QualType dstTy,
SourceLocation loc) {
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 76e9ca4fd61a8..5412f9f602711 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -513,6 +513,8 @@ LValue CIRGenFunction::emitLValue(const Expr *e) {
return emitArraySubscriptExpr(cast<ArraySubscriptExpr>(e));
case Expr::UnaryOperatorClass:
return emitUnaryOpLValue(cast<UnaryOperator>(e));
+ case Expr::MemberExprClass:
+ return emitMemberExpr(cast<MemberExpr>(e));
case Expr::BinaryOperatorClass:
return emitBinaryOperatorLValue(cast<BinaryOperator>(e));
case Expr::ParenExprClass:
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 01abd84ce1c85..f533d0ab53cd2 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -423,6 +423,10 @@ class CIRGenFunction : public CIRGenTypeCache {
clang::CharUnits alignment);
public:
+ Address emitAddrOfFieldStorage(Address base, const FieldDecl *field,
+ llvm::StringRef fieldName,
+ unsigned fieldIndex);
+
mlir::Value emitAlloca(llvm::StringRef name, mlir::Type ty,
mlir::Location loc, clang::CharUnits alignment,
bool insertIntoFnEntryBlock,
@@ -551,6 +555,9 @@ class CIRGenFunction : public CIRGenTypeCache {
/// of the expression.
/// FIXME: document this function better.
LValue emitLValue(const clang::Expr *e);
+ LValue emitLValueForField(LValue base, const clang::FieldDecl *field);
+
+ LValue emitMemberExpr(const MemberExpr *e);
/// Given an expression with a pointer type, emit the value and compute our
/// best estimate of the alignment of the pointee.
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 1e0d6623c4f40..1fb97334d7bd2 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -19,6 +19,7 @@
#include "CIRGenValue.h"
#include "clang/AST/CharUnits.h"
+#include "clang/CIR/Dialect/IR/CIRDataLayout.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "TargetInfo.h"
@@ -95,6 +96,12 @@ class CIRGenModule : public CIRGenTypeCache {
const clang::LangOptions &getLangOpts() const { return langOpts; }
mlir::MLIRContext &getMLIRContext() { return *builder.getContext(); }
+ const cir::CIRDataLayout getDataLayout() const {
+ // FIXME(cir): instead of creating a CIRDataLayout every time, set it as an
+ // attribute for the CIRModule class.
+ return cir::CIRDataLayout(theModule);
+ }
+
/// -------
/// Handling globals
/// -------
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
index a51e0460d1074..39a9d16ffd766 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
@@ -42,10 +42,10 @@ class CIRGenRecordLayout {
cir::RecordType getCIRType() const { return completeObjectType; }
/// Return cir::RecordType element number that corresponds to the field FD.
- unsigned getCIRFieldNo(const clang::FieldDecl *FD) const {
- FD = FD->getCanonicalDecl();
- assert(fieldInfo.count(FD) && "Invalid field for record!");
- return fieldInfo.lookup(FD);
+ unsigned getCIRFieldNo(const clang::FieldDecl *fd) const {
+ fd = fd->getCanonicalDecl();
+ assert(fieldInfo.count(fd) && "Invalid field for record!");
+ return fieldInfo.lookup(fd);
}
};
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index 7bd86cf0c7bcd..90993c71be9a6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -446,6 +446,27 @@ mlir::Type CIRGenTypes::convertTypeForMem(clang::QualType qualType,
return convertedType;
}
+/// Return record layout info for the given record decl.
+const CIRGenRecordLayout &
+CIRGenTypes::getCIRGenRecordLayout(const RecordDecl *rd) {
+ const auto *key = astContext.getTagDeclType(rd).getTypePtr();
+
+ // If we have already computed the layout, return it.
+ auto it = cirGenRecordLayouts.find(key);
+ if (it != cirGenRecordLayouts.end())
+ return *it->second;
+
+ // Compute the type information.
+ convertRecordDeclType(rd);
+
+ // Now try again.
+ it = cirGenRecordLayouts.find(key);
+
+ assert(it != cirGenRecordLayouts.end() &&
+ "Unable to find record layout information for type");
+ return *it->second;
+}
+
bool CIRGenTypes::isZeroInitializable(clang::QualType t) {
if (t->getAs<PointerType>())
return astContext.getTargetNullPointerValue(t) == 0;
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.h b/clang/lib/CIR/CodeGen/CIRGenTypes.h
index 2bb78420700f8..5b4027601ca3a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.h
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.h
@@ -108,6 +108,8 @@ class CIRGenTypes {
std::string getRecordTypeName(const clang::RecordDecl *,
llvm::StringRef suffix);
+ const CIRGenRecordLayout &getCIRGenRecordLayout(const clang::RecordDecl *rd);
+
/// Convert type T into an mlir::Type. This differs from convertType in that
/// it is used to convert to the memory representation for a type. For
/// example, the scalar representation for bool is i1, but the memory
diff --git a/clang/lib/CIR/CodeGen/CIRGenValue.h b/clang/lib/CIR/CodeGen/CIRGenValue.h
index d4d6f5a44622e..1c453dc9c86b5 100644
--- a/clang/lib/CIR/CodeGen/CIRGenValue.h
+++ b/clang/lib/CIR/CodeGen/CIRGenValue.h
@@ -140,6 +140,10 @@ class LValue {
// TODO: Add support for volatile
bool isVolatile() const { return false; }
+ unsigned getVRQualifiers() const {
+ return quals.getCVRQualifiers() & ~clang::Qualifiers::Const;
+ }
+
clang::QualType getType() const { return type; }
mlir::Value getPointer() const { return v; }
@@ -154,6 +158,9 @@ class LValue {
}
const clang::Qualifiers &getQuals() const { return quals; }
+ clang::Qualifiers &getQuals() { return quals; }
+
+ LValueBaseInfo getBaseInfo() const { return baseInfo; }
static LValue makeAddr(Address address, clang::QualType t,
LValueBaseInfo baseInfo) {
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index d2313e75870b4..fb85052b465f1 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -1096,6 +1096,24 @@ OpFoldResult cir::UnaryOp::fold(FoldAdaptor adaptor) {
return {};
}
+//===----------------------------------------------------------------------===//
+// GetMemberOp Definitions
+//===----------------------------------------------------------------------===//
+
+LogicalResult cir::GetMemberOp::verify() {
+ const auto recordTy = dyn_cast<RecordType>(getAddrTy().getPointee());
+ if (!recordTy)
+ return emitError() << "expected pointer to a record type";
+
+ if (recordTy.getMembers().size() <= getIndex())
+ return emitError() << "member index out of bounds";
+
+ if (recordTy.getMembers()[getIndex()] != getResultTy().getPointee())
+ return emitError() << "member type mismatch";
+
+ return mlir::success();
+}
+
//===----------------------------------------------------------------------===//
// TableGen'd op method definitions
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
index 8b5646f339bb3..c7ac02b7984a3 100644
--- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -309,6 +309,37 @@ RecordType::computeStructAlignment(const mlir::DataLayout &dataLayout) const {
return recordAlignment;
}
+uint64_t RecordType::getElementOffset(const ::mlir::DataLayout &dataLayout,
+ unsigned idx) const {
+ assert(idx < getMembers().size() && "acc...
[truncated]
|
erichkeane
reviewed
Apr 19, 2025
| member from the input record. | ||
|
|
||
| It expects a pointer to the base record as well as the name of the member | ||
| and its field index. |
Collaborator
There was a problem hiding this comment.
What does this do for members of inherited types? is that encoded via the record? Or are they 'flattened' and the index is consistent?
Does it seem sensible to have examples of inheritance?
One thing that might be interesting if we don't encode it, is sometimes casts between types are legal if they have a 'common initial sequence' depending on language mode and use... so I wonder if encoding that (which, debatably, can change the common-initial-sequence rules) is worth-while some-day. (This last part is lamenting/thinking, not necessariy immediately actionable).
Member
There was a problem hiding this comment.
Or are they 'flattened' and the index is consistent?
This one. For a bit of extra context: whenever something more complex is going on CIRGen takes care of it such that the final getmember is reliable.
One thing that might be interesting if we don't encode it is sometimes casts between types are legal if they have a 'common initial sequence' depending on language mode and use
Indeed interesting. If this turns out to help CIRGen / optimization we should be all in for it in the future. Any compelling use case in mind?
Collaborator
There was a problem hiding this comment.
There is a bunch of 'legal' vs 'UB' things with common-initial sequence, particularly in C that might be interesting to opt based on, but I don't have any good ideas.
Member
There was a problem hiding this comment.
gotcha, I usually try to keep a log of these ideas, I'll ad this one. If anything else come up let me know.
|
|
||
| unsigned offset = 0; | ||
|
|
||
| for (unsigned i = 0, e = idx; i != e; ++i) { |
Collaborator
There was a problem hiding this comment.
Is there good reason to not use range-for here? Something like:
for (mlir::Type ty : std::make_range(members.begin(), std::next(members.begin(), idx))) (though there is perhaps some ADT stuff to do a better job of this)
Member
There was a problem hiding this comment.
auto i : llvm::seq<unsigned>(0, idx)) could be a good one
Contributor
Author
There was a problem hiding this comment.
Is there good reason to not use range-for here? Something like:
for (mlir::Type ty : std::make_range(members.begin(), std::next(members.begin(), idx)))(though there is perhaps some ADT stuff to do a better job of this)
Given that it wasn't ending at the end, it seemed just as well to iterate like this. Is there an advantage to the range-for in this case, given the extra calls to set it up?
Collaborator
There was a problem hiding this comment.
Typically clang prefers range-for for readability and avoiding the 'error-prone-ness' of an index. Readability is because it assures that we are only visiting anything 'once', and the index is unchanged between iterations. Typically when I see one of these types of loops, it means "funny-business is happening in the body, so look closely".
xlauko
reviewed
Apr 19, 2025
| ```mlir | ||
| // Suppose we have a record with multiple members. | ||
| !s32i = !cir.int<s, 32> | ||
| !s8i = !cir.int<s, 32> |
Contributor
There was a problem hiding this comment.
Suggested change
| !s8i = !cir.int<s, 32> | |
| !s8i = !cir.int<s, 8> |
bcardosolopes
approved these changes
Apr 21, 2025
Member
bcardosolopes
left a comment
bcardosolopes
left a comment
There was a problem hiding this comment.
LGTM pending discussion/comments
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
erichkeane
approved these changes
Apr 22, 2025
This adds ClangIR support for accessing structure members. Access to union members is deferred to a later change.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/6457 Here is the relevant piece of the build log for the reference |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This adds ClangIR support for accessing structure members. Access to union members is deferred to a later change.
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.
This adds ClangIR support for accessing structure members. Access to union members is deferred to a later change.