-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[CIR] Add constant record ILE support #155663
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.
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
base: main
Are you sure you want to change the base?
Conversation
This patch adds basic support for constant record initializer list expressions. There's a couple of limitations: * No zero initialized padding bytes in C mode * No bitfields * No designated initializer lists * Record alignments are not calculated, yet * ILEs of derived records don't work, yet * The constant attribute is not propagated to the backend, resulting in non-constants being emitted in the LLVM IR
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Morris Hafner (mmha) ChangesThis patch adds basic support for constant record initializer list expressions. There's a couple of limitations:
Patch is 42.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/155663.diff 12 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
index 312d0a9422673..4eec34cb299ab 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -643,6 +643,8 @@ def CIR_RecordType : CIR_Type<"Record", "record", [
uint64_t getElementOffset(const mlir::DataLayout &dataLayout,
unsigned idx) const;
+ bool isLayoutIdentical(const RecordType &other);
+
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 d7a2e49ec162a..f0a818b388d51 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -134,6 +134,7 @@ struct MissingFeatures {
static bool astRecordDeclAttr() { return false; }
static bool cxxSupport() { return false; }
static bool recordZeroInit() { return false; }
+ static bool recordZeroInitPadding() { return false; }
static bool zeroSizeRecordMembers() { return false; }
static bool recordLayoutVirtualBases() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenBuilder.cpp
index 755c76c89a645..999c2fcbfa490 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "CIRGenBuilder.h"
+#include "mlir/IR/BuiltinAttributes.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/TypeSwitch.h"
using namespace clang::CIRGen;
@@ -130,6 +132,44 @@ void CIRGenBuilderTy::computeGlobalViewIndicesFromFlatOffset(
computeGlobalViewIndicesFromFlatOffset(offset, subType, layout, indices);
}
+static mlir::Type getAttributeType(mlir::Attribute attr) {
+ return mlir::cast<mlir::TypedAttr>(attr).getType();
+}
+
+cir::RecordType clang::CIRGen::CIRGenBuilderTy::getCompleteRecordType(
+ mlir::ArrayAttr fields, bool packed, bool padded, llvm::StringRef name,
+ const clang::RecordDecl *ast) {
+ llvm::SmallVector<mlir::Type, 8> members;
+ members.reserve(fields.size());
+ llvm::transform(fields, std::back_inserter(members), getAttributeType);
+
+ if (name.empty())
+ return getAnonRecordTy(members, packed, padded);
+
+ return getCompleteRecordTy(members, name, packed, padded);
+}
+
+mlir::Attribute clang::CIRGen::CIRGenBuilderTy::getConstRecordOrZeroAttr(
+ mlir::ArrayAttr arrayAttr, bool packed, bool padded, mlir::Type type) {
+ auto recordTy = mlir::cast_or_null<cir::RecordType>(type);
+
+ // Record type not specified: create anon record type from members.
+ if (!recordTy) {
+ llvm::SmallVector<mlir::Type, 8> members;
+ members.reserve(arrayAttr.size());
+ llvm::transform(arrayAttr, std::back_inserter(members), getAttributeType);
+ recordTy = getType<cir::RecordType>(members, packed, padded,
+ cir::RecordType::Struct);
+ }
+
+ // Return zero or anonymous constant record.
+ const bool isZero = llvm::all_of(
+ arrayAttr, [&](mlir::Attribute a) { return isNullValue(a); });
+ if (isZero)
+ return cir::ZeroAttr::get(recordTy);
+ return cir::ConstRecordAttr::get(recordTy, arrayAttr);
+}
+
// This can't be defined in Address.h because that file is included by
// CIRGenBuilder.h
Address Address::withElementType(CIRGenBuilderTy &builder,
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index d5cb6d416bc9b..23c046d6922cc 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -12,8 +12,10 @@
#include "Address.h"
#include "CIRGenRecordLayout.h"
#include "CIRGenTypeCache.h"
+#include "mlir/IR/Attributes.h"
+#include "mlir/IR/BuiltinAttributes.h"
+#include "mlir/Support/LLVM.h"
#include "clang/CIR/Dialect/IR/CIRDataLayout.h"
-#include "clang/CIR/Interfaces/CIRTypeInterfaces.h"
#include "clang/CIR/MissingFeatures.h"
#include "clang/CIR/Dialect/Builder/CIRBaseBuilder.h"
@@ -60,6 +62,16 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
trailingZerosNum);
}
+ cir::ConstArrayAttr getConstArray(mlir::Attribute attrs,
+ cir::ArrayType arrayTy) const {
+ return cir::ConstArrayAttr::get(arrayTy, attrs);
+ }
+
+ mlir::Attribute getConstRecordOrZeroAttr(mlir::ArrayAttr arrayAttr,
+ bool packed = false,
+ bool padded = false,
+ mlir::Type type = {});
+
cir::ConstRecordAttr getAnonConstRecord(mlir::ArrayAttr arrayAttr,
bool packed = false,
bool padded = false,
@@ -150,6 +162,12 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
return type;
}
+ cir::RecordType getCompleteRecordType(mlir::ArrayAttr fields,
+ bool packed = false,
+ bool padded = false,
+ llvm::StringRef name = "",
+ const clang::RecordDecl *ast = nullptr);
+
/// Get an incomplete CIR struct type. If we have a complete record
/// declaration, we may create an incomplete type and then add the
/// members, so \p rd here may be complete.
diff --git a/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
index d455f6e283406..7af62b9845e40 100644
--- a/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
+++ b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
@@ -81,6 +81,14 @@ class ConstantEmitter {
// side effects, or emitting an initialization that requires a
// reference to its current location.
mlir::Attribute emitForMemory(mlir::Attribute c, QualType destType);
+ static mlir::Attribute emitForMemory(CIRGenModule &cgm, mlir::Attribute c,
+ clang::QualType destTy);
+
+ mlir::Attribute emitNullForMemory(mlir::Location loc, QualType t) {
+ return emitNullForMemory(loc, cgm, t);
+ }
+ static mlir::Attribute emitNullForMemory(mlir::Location loc,
+ CIRGenModule &cgm, QualType t);
/// Try to emit the initializer of the given declaration as an abstract
/// constant.
@@ -104,7 +112,8 @@ class ConstantEmitter {
mlir::TypedAttr tryEmitPrivate(const Expr *e, QualType destType);
mlir::Attribute tryEmitPrivate(const APValue &value, QualType destType);
- mlir::Attribute tryEmitPrivateForMemory(const APValue &value, QualType t);
+ mlir::Attribute tryEmitPrivateForMemory(const Expr *e, QualType destTy);
+ mlir::Attribute tryEmitPrivateForMemory(const APValue &value, QualType destTy);
private:
#ifndef NDEBUG
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index 23132eae3214e..7af99f3282fc9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -12,7 +12,6 @@
#include "Address.h"
#include "CIRGenConstantEmitter.h"
-#include "CIRGenFunction.h"
#include "CIRGenModule.h"
#include "CIRGenRecordLayout.h"
#include "mlir/IR/Attributes.h"
@@ -21,20 +20,571 @@
#include "clang/AST/APValue.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/CharUnits.h"
#include "clang/AST/OperationKinds.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/Builtins.h"
-#include "clang/Basic/Specifiers.h"
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include "clang/CIR/MissingFeatures.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/Sequence.h"
#include "llvm/Support/ErrorHandling.h"
+#include <iterator>
using namespace clang;
using namespace clang::CIRGen;
+//===----------------------------------------------------------------------===//
+// ConstantAggregateBuilder
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ConstExprEmitter;
+
+static mlir::TypedAttr computePadding(CIRGenModule &cgm, CharUnits size) {
+ mlir::Type eltTy = cgm.UCharTy;
+ clang::CharUnits::QuantityType arSize = size.getQuantity();
+ CIRGenBuilderTy &bld = cgm.getBuilder();
+ if (size > CharUnits::One()) {
+ SmallVector<mlir::Attribute, 4> elts(arSize, cir::ZeroAttr::get(eltTy));
+ return bld.getConstArray(mlir::ArrayAttr::get(bld.getContext(), elts),
+ cir::ArrayType::get(eltTy, arSize));
+ }
+
+ return cir::ZeroAttr::get(eltTy);
+}
+
+static mlir::Attribute
+emitArrayConstant(CIRGenModule &cgm, mlir::Type desiredType,
+ mlir::Type commonElementType, unsigned arrayBound,
+ SmallVectorImpl<mlir::TypedAttr> &elements,
+ mlir::TypedAttr filler);
+
+struct ConstantAggregateBuilderUtils {
+ CIRGenModule &cgm;
+ cir::CIRDataLayout dataLayout;
+
+ ConstantAggregateBuilderUtils(CIRGenModule &cgm)
+ : cgm(cgm), dataLayout{cgm.getModule()} {}
+
+ CharUnits getAlignment(const mlir::TypedAttr c) const {
+ return CharUnits::fromQuantity(
+ dataLayout.getAlignment(c.getType(), /*abiOrPref=*/true));
+ }
+
+ CharUnits getSize(mlir::Type ty) const {
+ return CharUnits::fromQuantity(dataLayout.getTypeAllocSize(ty));
+ }
+
+ CharUnits getSize(const mlir::TypedAttr c) const {
+ return getSize(c.getType());
+ }
+
+ mlir::TypedAttr getPadding(CharUnits size) const {
+ return computePadding(cgm, size);
+ }
+};
+
+/// Incremental builder for an mlir::TypedAttr holding a record or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+ /// The elements of the constant. These two arrays must have the same size;
+ /// Offsets[i] describes the offset of Elems[i] within the constant. The
+ /// elements are kept in increasing offset order, and we ensure that there
+ /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+ ///
+ /// This may contain explicit padding elements (in order to create a
+ /// natural layout), but need not. Gaps between elements are implicitly
+ /// considered to be filled with undef.
+ llvm::SmallVector<mlir::TypedAttr, 32> elems;
+ llvm::SmallVector<CharUnits, 32> offsets;
+
+ /// The size of the constant (the maximum end offset of any added element).
+ /// May be larger than the end of Elems.back() if we split the last element
+ /// and removed some trailing undefs.
+ CharUnits size = CharUnits::Zero();
+
+ /// This is true only if laying out Elems in order as the elements of a
+ /// non-packed LLVM struct will give the correct layout.
+ bool naturalLayout = true;
+
+ std::optional<size_t> splitAt(CharUnits pos);
+
+ static mlir::Attribute
+ buildFrom(CIRGenModule &cgm, ArrayRef<mlir::TypedAttr> elems,
+ ArrayRef<CharUnits> offsets, CharUnits startOffset, CharUnits size,
+ bool naturalLayout, mlir::Type desiredTy, bool allowOversized);
+
+public:
+ ConstantAggregateBuilder(CIRGenModule &cgm)
+ : ConstantAggregateBuilderUtils(cgm) {}
+
+ /// Update or overwrite the value starting at \p offset with \c c.
+ ///
+ /// \param allowOverwrite If \c true, this constant might overwrite (part of)
+ /// a constant that has already been added. This flag is only used to
+ /// detect bugs.
+ bool add(mlir::TypedAttr typedAttr, CharUnits offset, bool allowOverwrite);
+
+ /// Update or overwrite the bits starting at \p offsetInBits with \p bits.
+ bool addBits(llvm::APInt bits, uint64_t offsetInBits, bool allowOverwrite);
+
+ /// Produce a constant representing the entire accumulated value, ideally of
+ /// the specified type. If \p allowOversized, the constant might be larger
+ /// than implied by \p desiredTy (eg, if there is a flexible array member).
+ /// Otherwise, the constant will be of exactly the same size as \p desiredTy
+ /// even if we can't represent it as that type.
+ mlir::Attribute build(mlir::Type desiredTy, bool allowOversized) const {
+ return buildFrom(cgm, elems, offsets, CharUnits::Zero(), size,
+ naturalLayout, desiredTy, allowOversized);
+ }
+};
+
+template <typename Container, typename Range = std::initializer_list<
+ typename Container::value_type>>
+static void replace(Container &c, size_t beginOff, size_t endOff, Range vals) {
+ assert(beginOff <= endOff && "invalid replacement range");
+ llvm::replace(c, c.begin() + beginOff, c.begin() + endOff, vals);
+}
+
+bool ConstantAggregateBuilder::add(mlir::TypedAttr typedAttr, CharUnits offset,
+ bool allowOverwrite) {
+ // Common case: appending to a layout.
+ if (offset >= size) {
+ CharUnits align = getAlignment(typedAttr);
+ CharUnits alignedSize = size.alignTo(align);
+ if (alignedSize > offset || offset.alignTo(align) != offset)
+ naturalLayout = false;
+ else if (alignedSize < offset) {
+ elems.push_back(getPadding(offset - size));
+ offsets.push_back(size);
+ }
+ elems.push_back(typedAttr);
+ offsets.push_back(offset);
+ size = offset + getSize(typedAttr);
+ return true;
+ }
+
+ // Uncommon case: constant overlaps what we've already created.
+ std::optional<size_t> firstElemToReplace = splitAt(offset);
+ if (!firstElemToReplace)
+ return false;
+
+ CharUnits cSize = getSize(typedAttr);
+ std::optional<size_t> lastElemToReplace = splitAt(offset + cSize);
+ if (!lastElemToReplace)
+ return false;
+
+ assert((firstElemToReplace == lastElemToReplace || allowOverwrite) &&
+ "unexpectedly overwriting field");
+
+ replace(elems, *firstElemToReplace, *lastElemToReplace, {typedAttr});
+ replace(offsets, *firstElemToReplace, *lastElemToReplace, {offset});
+ size = std::max(size, offset + cSize);
+ naturalLayout = false;
+ return true;
+}
+
+/// Returns a position within Elems and Offsets such that all elements
+/// before the returned index end before Pos and all elements at or after
+/// the returned index begin at or after Pos. Splits elements as necessary
+/// to ensure this. Returns None if we find something we can't split.
+std::optional<size_t> ConstantAggregateBuilder::splitAt(CharUnits pos) {
+ if (pos >= size)
+ return offsets.size();
+
+ clang::CharUnits *firstAfterPos = llvm::upper_bound(offsets, pos);
+ if (firstAfterPos == offsets.begin())
+ return 0;
+
+ // If we already have an element starting at Pos, we're done.
+ size_t lastAtOrBeforePosIndex = firstAfterPos - offsets.begin() - 1;
+ if (offsets[lastAtOrBeforePosIndex] == pos)
+ return lastAtOrBeforePosIndex;
+
+ // We found an element starting before Pos. Check for overlap.
+ mlir::TypedAttr c =
+ mlir::cast<mlir::TypedAttr>(elems[lastAtOrBeforePosIndex]);
+ if (offsets[lastAtOrBeforePosIndex] + getSize(c) <= pos)
+ return lastAtOrBeforePosIndex + 1;
+
+ // Try to decompose it into smaller constants.
+ cgm.errorNYI("split into smaller constants");
+ return std::nullopt;
+}
+
+mlir::Attribute ConstantAggregateBuilder::buildFrom(
+ CIRGenModule &cgm, ArrayRef<mlir::TypedAttr> elems,
+ ArrayRef<CharUnits> offsets, CharUnits startOffset, CharUnits size,
+ bool naturalLayout, mlir::Type desiredTy, bool allowOversized) {
+ ConstantAggregateBuilderUtils utils(cgm);
+
+ if (elems.empty())
+ return cir::UndefAttr::get(desiredTy);
+
+ auto offset = [&](size_t i) { return offsets[i] - startOffset; };
+
+ // If we want an array type, see if all the elements are the same type and
+ // appropriately spaced.
+ if (mlir::isa<cir::ArrayType>(desiredTy)) {
+ cgm.errorNYI("array aggregate constants");
+ return {};
+ }
+
+ // The size of the constant we plan to generate. This is usually just the size
+ // of the initialized type, but in AllowOversized mode (i.e. flexible array
+ // init), it can be larger.
+ CharUnits desiredSize = utils.getSize(desiredTy);
+ if (size > desiredSize) {
+ assert(allowOversized && "Elems are oversized");
+ desiredSize = size;
+ }
+
+ // The natural alignment of an unpacked CIR record with the given elements.
+ CharUnits align = CharUnits::One();
+ for (mlir::TypedAttr e : elems) {
+ align = std::max(align, utils.getAlignment(e));
+ }
+
+ // The natural size of an unpacked LLVM struct with the given elements.
+ CharUnits alignedSize = size.alignTo(align);
+
+ bool packed = false;
+ bool padded = false;
+ ArrayRef<mlir::TypedAttr> unpackedElems = elems;
+
+ llvm::SmallVector<mlir::TypedAttr, 32> unpackedElemStorage;
+ if (desiredSize < alignedSize || desiredSize.alignTo(align) != desiredSize) {
+ naturalLayout = false;
+ packed = true;
+ } else if (desiredSize > alignedSize) {
+ // The natural layout would be too small. Add padding to fix it. (This
+ // is ignored if we choose a packed layout.)
+ unpackedElemStorage.assign(unpackedElems.begin(), unpackedElems.end());
+ unpackedElemStorage.push_back(utils.getPadding(desiredSize - size));
+ unpackedElems = unpackedElemStorage;
+ }
+
+ // If we don't have a natural layout, insert padding as necessary.
+ // As we go, double-check to see if we can actually just emit Elems
+ // as a non-packed record and do so opportunistically if possible.
+ llvm::SmallVector<mlir::TypedAttr, 32> packedElems;
+ if (!naturalLayout) {
+ CharUnits sizeSoFar = CharUnits::Zero();
+ for (auto [index, element] : llvm::enumerate(elems)) {
+ CharUnits align = utils.getAlignment(element);
+ CharUnits naturalOffset = sizeSoFar.alignTo(align);
+ CharUnits desiredOffset = offset(index);
+ assert(desiredOffset >= sizeSoFar && "elements out of order");
+
+ if (desiredOffset != naturalOffset)
+ packed = true;
+ if (desiredOffset != sizeSoFar)
+ packedElems.push_back(utils.getPadding(desiredOffset - sizeSoFar));
+ packedElems.push_back(element);
+ sizeSoFar = desiredOffset + utils.getSize(element);
+ }
+ // If we're using the packed layout, pad it out to the desired size if
+ // necessary.
+ if (packed) {
+ assert(sizeSoFar <= desiredSize &&
+ "requested size is too small for contents");
+
+ if (sizeSoFar < desiredSize)
+ packedElems.push_back(utils.getPadding(desiredSize - sizeSoFar));
+ }
+ }
+
+ CIRGenBuilderTy &builder = cgm.getBuilder();
+ llvm::SmallVector<mlir::Attribute, 32> arrayElements;
+ arrayElements.reserve(elems.size());
+ if (packed)
+ llvm::copy(packedElems, std::back_inserter(arrayElements));
+ else
+ llvm::copy(unpackedElems, std::back_inserter(arrayElements));
+ auto arrAttr = mlir::ArrayAttr::get(builder.getContext(), arrayElements);
+
+ cir::RecordType strType = builder.getCompleteRecordType(arrAttr, packed);
+ if (auto desired = mlir::dyn_cast<cir::RecordType>(desiredTy))
+ if (desired.isLayoutIdentical(strType))
+ strType = desired;
+
+ return builder.getConstRecordOrZeroAttr(arrAttr, packed, padded, strType);
+}
+
+//===----------------------------------------------------------------------===//
+// ConstRecordBuilder
+//===----------------------------------------------------------------------===//
+
+class ConstRecordBuilder {
+ CIRGenModule &cgm;
+ ConstantEmitter &emitter;
+ ConstantAggregateBuilder &builder;
+ CharUnits startOffset;
+
+public:
+ static mlir::Attribute buildRecord(ConstantEmitter &emitter,
+ InitListExpr *ile, QualType valTy);
+ static mlir::Attribute buildRecord(ConstantEmitter &emitter,
+ const APValue &value, QualType valTy);
+ static bool updateRecord(ConstantEmitter &emitter,
+ ConstantAggregateBuilder &constant, CharUnits offset,
+ InitListExpr *updater);
+
+private:
+ ConstRecordBuilder(ConstantEmitter &emitter,
+ ConstantAggregateBuilder &builder, CharUnits startOffset)
+ : cgm(emitter.cgm), emitter(emitter), builder(builder),
+ startOffset(startOffset) {}
+
+ bool appendField(const Fi...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
||
using namespace clang; | ||
using namespace clang::CIRGen; | ||
|
||
//===----------------------------------------------------------------------===// | ||
// ConstantAggregateBuilder | ||
//===----------------------------------------------------------------------===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The amount of code added in this file doesn't seem to match the number of added tests (many if
legs to account for), are they all exercised or am I missing something?. One example that seems to be missing tests is for packed
.
Any way this could be more incremental / easier to review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple more test cases for a very simple struct, packed and packed + overly aligned. That should give us a much better coverage of what's supposed to work with this PR.
I could remove the bits for everything that's not a natural layout but that wouldn't save much code IMO.
@@ -30,7 +30,8 @@ llvm::Align CIRDataLayout::getAlignment(mlir::Type ty, bool useABIAlign) const { | |||
return llvm::Align(1); | |||
|
|||
// Get the layout annotation... which is lazily created on demand. | |||
llvm_unreachable("getAlignment()) for record type is not implemented"); | |||
assert(!cir::MissingFeatures::alignCXXRecordDecl()); | |||
return llvm::Align(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #155721 I'm proposing to delete the special handling here for record types entirely because I think the call to layout.getTypeABIAlignment()
or layout.getTypePreferredAlignment()
should handle record types correctly (and if it doesn't we should be able to fix that by providing a handler in CIRTypes.cpp
.
Can you try that and see if it works (i.e. produces the same alignment as OGCG) for your change?
clang::CharUnits::QuantityType arSize = size.getQuantity(); | ||
CIRGenBuilderTy &bld = cgm.getBuilder(); | ||
if (size > CharUnits::One()) { | ||
SmallVector<mlir::Attribute, 4> elts(arSize, cir::ZeroAttr::get(eltTy)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SmallVector<mlir::Attribute, 4> elts(arSize, cir::ZeroAttr::get(eltTy)); | |
SmallVector<mlir::Attribute> elts(arSize, cir::ZeroAttr::get(eltTy)); |
|
||
CharUnits getAlignment(const mlir::TypedAttr c) const { | ||
return CharUnits::fromQuantity( | ||
dataLayout.getAlignment(c.getType(), /*abiOrPref=*/true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dataLayout.getAlignment(c.getType(), /*abiOrPref=*/true)); | |
dataLayout.getAlignment(c.getType(), /*useABIAlign=*/true)); |
abiOrPref=true
has a real "if you come to a fork in the road, take it" quality to it. Fortunately, the actual parameter name has more clarity.
/// constant. | ||
class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils { | ||
/// The elements of the constant. These two arrays must have the same size; | ||
/// Offsets[i] describes the offset of Elems[i] within the constant. The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Offsets[i] describes the offset of Elems[i] within the constant. The | |
/// offsets[i] describes the offset of elems[i] within the constant. The |
/// The elements of the constant. These two arrays must have the same size; | ||
/// Offsets[i] describes the offset of Elems[i] within the constant. The | ||
/// elements are kept in increasing offset order, and we ensure that there | ||
/// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]). | |
/// is no overlap: offsets[i+1] >= offsets[i] + getSize(elems[i]). |
|
||
// Accumulate and sort bases, in order to visit them in address order, which | ||
// may not be the same as declaration order. | ||
SmallVector<BaseInfo, 8> bases; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SmallVector<BaseInfo, 8> bases; | |
SmallVector<BaseInfo> bases; |
CharUnits baseOffset = layout.getBaseClassOffset(bd); | ||
bases.push_back(BaseInfo(bd, baseOffset, index)); | ||
} | ||
llvm::stable_sort(bases); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be a bit surprised if these aren't already correctly sorted. Have you seen otherwise?
SmallVector<BaseInfo, 8> bases; | ||
bases.reserve(cd->getNumBases()); | ||
for (auto [index, base] : llvm::enumerate(cd->bases())) { | ||
assert(!base.isVirtual() && "should not have virtual bases here"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this prevented?
CharUnits offset) { | ||
const ASTRecordLayout &layout = cgm.getASTContext().getASTRecordLayout(rd); | ||
if (const CXXRecordDecl *cd = dyn_cast<CXXRecordDecl>(rd)) { | ||
assert(!cir::MissingFeatures::vtableInitialization()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems more like it should be an errorNYI
if the class requires vtable initialization.
@@ -903,8 +1440,7 @@ mlir::Attribute ConstantEmitter::tryEmitPrivate(const APValue &value, | |||
return ConstantLValueEmitter(*this, value, destType).tryEmit(); | |||
case APValue::Struct: | |||
case APValue::Union: | |||
cgm.errorNYI("ConstExprEmitter::tryEmitPrivate struct or union"); | |||
return {}; | |||
return ConstRecordBuilder::buildRecord(*this, value, destType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which case gets us here? Is this covered by one of the tests?
This patch adds basic support for constant record initializer list expressions. There's a couple of limitations: