https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/216349
>From 8a309bbbe03a84d654130de6c54777602ba1a1ee Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Thu, 13 Aug 2026 14:54:35 -0700 Subject: [PATCH 1/2] [CIR] Correct union lowering behavior re-padding Most of the code that CIR lowers to LLVM counts on the fact that our alignnments are correct/calculated in LLVM to get our layout correctly. This works for the most part, and unions have the storage type of the 'highest' alignment type. However, when creating a constant, we have to convert the type of the union to have a 'storage' type that matches the data being inserted (not the union's storage type!). The result was that if we had a storage type where the alignment was smaller than the actual storage type, LLVM would mis-calculate the padding. This patch adds the padding explicitly when we make that conversion to get the alignment set up correctly. Note: there is one mild IR-equivilency-regression to this patch. There isn't really a great way to tell the difference between a union-tail-padding needing zero-init vs undef-init in this case. This patch chooses to make it always zero-init, which is harmless. While it MIGHT suppress some optimizations (facts not in evidence?), it seems like something we can figure out later if necessary. --- clang/include/clang/CIR/LoweringHelpers.h | 8 ++ .../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 17 ++- clang/lib/CIR/Lowering/LoweringHelpers.cpp | 125 +++++++++++++++--- clang/test/CIR/CodeGen/bitfields.cpp | 2 +- clang/test/CIR/CodeGen/union-agg-init.c | 59 ++++++++- clang/test/CIR/CodeGen/union-agg-init.cpp | 14 ++ .../CIR/CodeGen/unions-with-zero-init.cpp | 6 +- 7 files changed, 201 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/CIR/LoweringHelpers.h b/clang/include/clang/CIR/LoweringHelpers.h index 92633c89e1369..f0f65bb317521 100644 --- a/clang/include/clang/CIR/LoweringHelpers.h +++ b/clang/include/clang/CIR/LoweringHelpers.h @@ -54,6 +54,14 @@ mlir::Type adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, const mlir::TypeConverter &converter, const mlir::DataLayout &dataLayout); +// A version of adjustGlobalTypeForInit which records where additional padding +// was added in the middle, so we can properly adjust field indexes. +mlir::Type +adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, + const mlir::TypeConverter &converter, + const mlir::DataLayout &dataLayout, + llvm::SmallVectorImpl<unsigned> &paddingAddedIndexes); + mlir::Value getConstAPInt(mlir::OpBuilder &bld, mlir::Location loc, mlir::Type typ, const llvm::APInt &val); diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index 4ae4b7693d982..7986227a2d035 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -825,15 +825,26 @@ mlir::Value CIRAttrToValue::visitCirAttr(cir::ConstArrayAttr attr) { mlir::Value CIRAttrToValue::visitCirAttr(cir::ConstRecordAttr constRecord) { mlir::Type llvmTy = converter->convertType(constRecord.getType()); mlir::DataLayout dataLayout(parentOp->getParentOfType<mlir::ModuleOp>()); - llvmTy = adjustGlobalTypeForInit(llvmTy, constRecord, *converter, dataLayout); + llvm::SmallVector<unsigned> paddingAddedIndexes; + llvmTy = adjustGlobalTypeForInit(llvmTy, constRecord, *converter, dataLayout, + paddingAddedIndexes); const mlir::Location loc = parentOp->getLoc(); mlir::Value result = mlir::LLVM::UndefOp::create(rewriter, loc, llvmTy); + uint64_t insertIdx = 0; + auto paddingItr = paddingAddedIndexes.begin(); + // Iteratively lower each constant element of the record. for (auto [idx, elt] : llvm::enumerate(constRecord.getMembers())) { + if (paddingItr != paddingAddedIndexes.end() && *paddingItr == idx) { + ++insertIdx; + ++paddingItr; + } + mlir::Value init = visit(elt); - result = - mlir::LLVM::InsertValueOp::create(rewriter, loc, result, init, idx); + result = mlir::LLVM::InsertValueOp::create(rewriter, loc, result, init, + insertIdx); + ++insertIdx; } return result; diff --git a/clang/lib/CIR/Lowering/LoweringHelpers.cpp b/clang/lib/CIR/Lowering/LoweringHelpers.cpp index 3b927ad759c7a..db6448ee0023f 100644 --- a/clang/lib/CIR/Lowering/LoweringHelpers.cpp +++ b/clang/lib/CIR/Lowering/LoweringHelpers.cpp @@ -416,13 +416,23 @@ static bool shouldPackFAMStruct(const mlir::DataLayout &dataLayout, // Additionally, the struct itself could contain a struct with a FAM or a union // that needed adjustment, so it recurses to check those. If no such type has // been found/no adjustment needed, this returns the type unchanged. +// +// Additionally, a union having an active member of the not-the-largest +// alignment can cause the need for a small padding array. We also capture the +// original indices of the fields that had this padding prepended, so the +// lowerConstRecordAttr can later put in a 'zero' init there. static mlir::Type adjustGlobalStructTypeForInit( mlir::LLVM::LLVMStructType structTy, cir::ConstRecordAttr constRecord, - const mlir::TypeConverter &converter, const mlir::DataLayout &dataLayout) { - + const mlir::TypeConverter &converter, const mlir::DataLayout &dataLayout, + llvm::SmallVectorImpl<unsigned> &paddingAddedIndexes) { + assert(paddingAddedIndexes.empty() && + "Not for accumulation, just single depth"); llvm::ArrayRef<mlir::Attribute> initMembers = constRecord.getMembers().getValue(); - llvm::SmallVector<mlir::Type> newBody{structTy.getBody()}; + llvm::SmallVector<mlir::Type> origBody{structTy.getBody()}; + llvm::SmallVector<mlir::Type> newBody{}; + bool packed = structTy.isPacked(); + uint64_t curOffset = 0; bool changed = false; // Recursively adjust each member. A member that is itself a union (or a @@ -430,14 +440,45 @@ static mlir::Type adjustGlobalStructTypeForInit( // field type, and this struct has to adopt that adjusted type so the // enclosing insertvalue chain type-checks. for (auto [idx, member] : llvm::enumerate(initMembers)) { - if (idx >= newBody.size()) + if (idx >= origBody.size()) break; - mlir::Type adjusted = - adjustGlobalTypeForInit(newBody[idx], member, converter, dataLayout); - if (adjusted != newBody[idx]) { - newBody[idx] = adjusted; + mlir::Type adjusted = adjustGlobalTypeForInit( + origBody[idx], member, converter, dataLayout); + unsigned adjustedAlign = dataLayout.getTypeABIAlignment(adjusted); + + if (adjusted != origBody[idx]) { + // We're always going to 'change' the layout if it has changed, but we + // need to see if there is new 'padding' that won't happen automatically + // here based on alignment. + unsigned origAlign = dataLayout.getTypeABIAlignment(origBody[idx]); + + uint64_t origOffset = + packed ? curOffset : llvm::alignTo(curOffset, origAlign); + uint64_t adjustedOffset = + packed ? curOffset : llvm::alignTo(curOffset, adjustedAlign); + + if (adjustedOffset != origOffset) { + // If the offset would change, we have to insert padding to make up for + // it. This should only happen since alignment will decrease with + // unions, so we should be able to assume adjusted-offset < origOffset? + assert(adjustedOffset < origOffset); + // Rather than just pad the difference between the offsets, we have to + // fill in since the end of the last field, else we leave room thanks to + // alignment between this field and the padding. + uint64_t difference = origOffset - curOffset; + newBody.push_back(mlir::LLVM::LLVMArrayType::get( + mlir::IntegerType::get(structTy.getContext(), 8), + difference)); + paddingAddedIndexes.push_back(idx); + curOffset = origOffset; + } changed = true; } + newBody.push_back(adjusted); + + if (!packed) + curOffset = llvm::alignTo(curOffset, adjustedAlign); + curOffset += dataLayout.getTypeSize(adjusted).getFixedValue(); } // CIR supports flexible-array-members in its struct types. That is, a @@ -446,7 +487,7 @@ static mlir::Type adjustGlobalStructTypeForInit( // these, and our verifier allows it. However, the LLVM implementation does // NOT permit this, so we widen that trailing member to the initializer's // array type (packing the struct if that changes the layout). - bool packed = structTy.isPacked(); + bool widenedFAM = false; if (auto fam = mlir::dyn_cast<mlir::LLVM::LLVMArrayType>(structTy.getBody().back()); fam && fam.getNumElements() == 0) { @@ -455,6 +496,7 @@ static mlir::Type adjustGlobalStructTypeForInit( if (mlir::cast<cir::ArrayType>(lastInitType).getSize() != 0) { newBody.back() = converter.convertType(lastInitType); packed = packed || shouldPackFAMStruct(dataLayout, newBody); + widenedFAM = true; changed = true; } } @@ -462,6 +504,19 @@ static mlir::Type adjustGlobalStructTypeForInit( if (!changed) return structTy; + // We've likely reduced the alignment, so make sure we put padding 'behind' + // it. We can skip this in the FAM case, since a Flexible array member is not + // allowed to be initialized unless it is the 'last' element. So it doesn't + // need to be padded out. + if (!widenedFAM) { + uint64_t declaredSize = dataLayout.getTypeSize(structTy).getFixedValue(); + assert(curOffset <= declaredSize && "body bigger than type?"); + if (curOffset < declaredSize) + newBody.push_back(mlir::LLVM::LLVMArrayType::get( + mlir::IntegerType::get(structTy.getContext(), 8), + declaredSize - curOffset)); + } + return mlir::LLVM::LLVMStructType::getLiteral(structTy.getContext(), newBody, packed); } @@ -515,9 +570,11 @@ static mlir::Type adjustGlobalUnionTypeForInit( } // Apply various adjustments required for struct/union types. -mlir::Type adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, - const mlir::TypeConverter &converter, - const mlir::DataLayout &dataLayout) { +mlir::Type +adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, + const mlir::TypeConverter &converter, + const mlir::DataLayout &dataLayout, + llvm::SmallVectorImpl<unsigned> &paddingAddedIndexes) { // Conversions for both only happen if we have a record init. auto constRecord = mlir::dyn_cast_if_present<cir::ConstRecordAttr>(init); if (!constRecord) @@ -532,16 +589,25 @@ mlir::Type adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, // Structs can have a flexible array member, adjust that. if (mlir::isa<cir::StructType>(constRecord.getType())) return adjustGlobalStructTypeForInit(structTy, constRecord, converter, - dataLayout); + dataLayout, paddingAddedIndexes); if (mlir::isa<cir::UnionType>(constRecord.getType())) return adjustGlobalUnionTypeForInit(structTy, constRecord, converter, dataLayout); return llvmType; } +mlir::Type adjustGlobalTypeForInit(mlir::Type llvmType, mlir::Attribute init, + const mlir::TypeConverter &converter, + const mlir::DataLayout &dataLayout) { + llvm::SmallVector<unsigned> ignoredAddedIndexes; + return adjustGlobalTypeForInit(llvmType, init, converter, dataLayout, + ignoredAddedIndexes); +} + std::optional<mlir::Attribute> lowerConstRecordAttr( cir::ConstRecordAttr constRecord, mlir::SymbolTableCollection &symbolTables, const mlir::TypeConverter *converter, mlir::ModuleOp moduleOp) { + // Build one constant attribute per record member. The LLVM dialect global // translation accepts an ArrayAttr (one element per struct field) and emits // an llvm::ConstantStruct, so the whole initializer can be a single @@ -558,18 +624,37 @@ std::optional<mlir::Attribute> lowerConstRecordAttr( } // The lowered LLVM type may have more fields than the CIR record has members - // -- e.g. a union lowers to { active-member, [pad x i8] } (see - // adjustGlobalTypeForInit, the single source of truth for the shape). Fill - // any such synthesized (padding) fields with undef so this ArrayAttr has - // exactly one entry per LLVM field, matching the type the global is declared - // with. + // for a few reasons: + // 1- a union lowers to { active-member, [pad x i8]). + // 2- A struct that contains such a union can have its alignment changed too, + // so it needs tail padding to fill that in. + // 3- A struct containing a union whose initializer doesn't use the highest-aligned + // field will have to prepend a bit of padding, such as struct { i32, union { + // i64, i32 } }. Typically the union gets lowered to a struct { i64 } (as i64 + // has the greatest alignment), but if the init causes it to be the i32(or any + // such smaller field) we have to prepend it with padding: + // struct { i32, [4 x i8], struct { i32 }} + // instead of (with no init): + // struct { i32, struct { i64 }} + llvm::SmallVector<unsigned> paddingAddedIndexes; mlir::Type adjustedTy = adjustGlobalTypeForInit( converter->convertType(constRecord.getType()), constRecord, *converter, - mlir::DataLayout(moduleOp)); + mlir::DataLayout(moduleOp), paddingAddedIndexes); + + // This handles #3 from above. adjustGlobalTypeForInit ensures the + // indexes are in increasing order, so we can insert 'backwards' without + // causing problems. + for (unsigned paddedElt : llvm::reverse(paddingAddedIndexes)) + loweredMembers.insert(loweredMembers.begin() + paddedElt, + mlir::LLVM::ZeroAttr::get(constRecord.getContext())); + + // Any remaining difference will be the union/struct padding case. We don't + // have a great handle/way to tell when to zero-vs-undef init, so always + // zero init, as it is always safe to do so. if (auto structTy = mlir::dyn_cast<mlir::LLVM::LLVMStructType>(adjustedTy)) while (loweredMembers.size() < structTy.getBody().size()) loweredMembers.push_back( - mlir::LLVM::UndefAttr::get(constRecord.getContext())); + mlir::LLVM::ZeroAttr::get(constRecord.getContext())); return mlir::ArrayAttr::get(constRecord.getContext(), loweredMembers); } diff --git a/clang/test/CIR/CodeGen/bitfields.cpp b/clang/test/CIR/CodeGen/bitfields.cpp index f1d2561dc2e44..7dce9bf29cd4a 100644 --- a/clang/test/CIR/CodeGen/bitfields.cpp +++ b/clang/test/CIR/CodeGen/bitfields.cpp @@ -32,7 +32,7 @@ typedef struct { union U { int x : 3; }; const U u = {5}; // CIR-DAG: cir.global "private" {{.*}}@_ZL1u = #cir.const_record<{#cir.int<5> : !u8i}> : !rec_U -// LLVM-DAG: @_ZL1u = internal constant %union.U { i8 5, [3 x i8] undef } +// LLVM-DAG: @_ZL1u = internal constant %union.U { i8 5, [3 x i8] zeroinitializer } // OGCG-DAG: @_ZL1u = internal constant %union.U { i8 5, [3 x i8] undef } auto use() { return u; diff --git a/clang/test/CIR/CodeGen/union-agg-init.c b/clang/test/CIR/CodeGen/union-agg-init.c index ab603d24d8712..ca390d25461d6 100644 --- a/clang/test/CIR/CodeGen/union-agg-init.c +++ b/clang/test/CIR/CodeGen/union-agg-init.c @@ -5,15 +5,67 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll // RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s +union PtrToIntUnion { int id; char *str; }; +struct HasPtoIU { int info; union PtrToIntUnion u; }; +struct HasPtoIU ptoIU = { 101, { 1 } }; +// CIR-DAG: cir.global external @ptoIU = #cir.const_record<{#cir.int<101> : !s32i, #cir.const_record<{#cir.int<1> : !s32i}> : !rec_PtrToIntUnion}> : !rec_HasPtoIU +// LLVM-DAG: @ptoIU = global { i32, [4 x i8], { i32, [4 x i8] } } { i32 101, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 1, [4 x i8] zeroinitializer } } + +struct WithTailPadding { int info; union PtrToIntUnion u; int tail; }; +struct WithTailPadding tailPadding = { 101, { 1 }, 42 }; +// CIR-DAG: cir.global external @tailPadding = #cir.const_record<{#cir.int<101> : !s32i, #cir.const_record<{#cir.int<1> : !s32i}> : !rec_PtrToIntUnion, #cir.int<42> : !s32i}> : !rec_WithTailPadding +// LLVM-DAG: @tailPadding = global { i32, [4 x i8], { i32, [4 x i8] }, i32, [4 x i8] } { i32 101, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 1, [4 x i8] zeroinitializer }, i32 42, [4 x i8] zeroinitializer } + +struct AtStart { union PtrToIntUnion u; int x; }; +struct AtStart start = { 7, 9 }; +// CIR-DAG: cir.global external @start = #cir.const_record<{#cir.const_record<{#cir.int<7> : !s32i}> : !rec_PtrToIntUnion, #cir.int<9> : !s32i}> : !rec_AtStart +// LLVM-DAG: @start = global { { i32, [4 x i8] }, i32, [4 x i8] } { { i32, [4 x i8] } { i32 7, [4 x i8] zeroinitializer }, i32 9, [4 x i8] zeroinitializer } + +struct NotToplevel { char c; struct WithTailPadding inner; }; +struct NotToplevel notTop = { 'x', { 101, { 1 }, 42 } }; +// CIR-DAG: cir.global external @notTop = #cir.const_record<{#cir.int<120> : !s8i, #cir.const_record<{#cir.int<101> : !s32i, #cir.const_record<{#cir.int<1> : !s32i}> : !rec_PtrToIntUnion, #cir.int<42> : !s32i}> : !rec_WithTailPadding}> : !rec_NotToplevel +// LLVM-DAG: @notTop = global { i8, [7 x i8], { i32, [4 x i8], { i32, [4 x i8] }, i32, [4 x i8] } } { i8 120, [7 x i8] zeroinitializer, { i32, [4 x i8], { i32, [4 x i8] }, i32, [4 x i8] } { i32 101, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 1, [4 x i8] zeroinitializer }, i32 42, [4 x i8] zeroinitializer } } + +struct TwoUnions { + int a; + union PtrToIntUnion u1; + int b; + union PtrToIntUnion u2; +}; +struct TwoUnions two_unions = { 1, { 2 }, 3, { 4 } }; +// CIR-DAG: cir.global external @two_unions = #cir.const_record<{#cir.int<1> : !s32i, #cir.const_record<{#cir.int<2> : !s32i}> : !rec_PtrToIntUnion, #cir.int<3> : !s32i, #cir.const_record<{#cir.int<4> : !s32i}> : !rec_PtrToIntUnion}> : !rec_TwoUnions +// LLVM-DAG: @two_unions = global { i32, [4 x i8], { i32, [4 x i8] }, i32, [4 x i8], { i32, [4 x i8] } } { i32 1, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 2, [4 x i8] zeroinitializer }, i32 3, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 4, [4 x i8] zeroinitializer } } + +struct Anon { int info; union { int id; char *str; } u; }; +struct Anon anon = { 101, 1 }; +// CIR-DAG: cir.global external @anon = #cir.const_record<{#cir.int<101> : !s32i, #cir.const_record<{#cir.int<1> : !s32i}> : !rec_anon2E0}> : !rec_Anon +// LLVM-DAG: @anon = global { i32, [4 x i8], { i32, [4 x i8] } } { i32 101, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 1, [4 x i8] zeroinitializer } } + +struct Bitfields { int a : 3; int b : 4; union PtrToIntUnion u; }; +struct Bitfields bitfields = { 1, 2, { 9 } }; +// CIR-DAG: cir.global external @bitfields = #cir.const_record<{#cir.int<17> : !u8i, #cir.const_record<{#cir.int<9> : !s32i}> : !rec_PtrToIntUnion}> : !rec_Bitfields {alignment = 8 : i64} loc(#loc42) +// LLVM-DAG: @bitfields = global { i8, [7 x i8], { i32, [4 x i8] } } { i8 17, [7 x i8] zeroinitializer, { i32, [4 x i8] } { i32 9, [4 x i8] zeroinitializer } } + +struct FamUnion { int n; union PtrToIntUnion u; char fam[]; }; +struct FamUnion fam_union = { 3, { 7 }, { 'a','b','c' } }; +// CIR-DAG: cir.global external @fam_union = #cir.const_record<{#cir.int<3> : !s32i, #cir.const_record<{#cir.int<7> : !s32i}> : !rec_PtrToIntUnion, #cir.const_array<[#cir.int<97> : !s8i, #cir.int<98> : !s8i, #cir.int<99> : !s8i]> : !cir.array<!s8i x 3>}> : !rec_FamUnion +// LLVM-DAG: @fam_union = global <{ i32, [4 x i8], { i32, [4 x i8] }, [3 x i8] }> <{ i32 3, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 7, [4 x i8] zeroinitializer }, [3 x i8] c"abc" }> + +struct FamMoves { char c; union PtrToIntUnion u; char fam[]; }; +struct FamMoves fam_realign = { 'q', { 7 }, { 'a','b','c' } }; +// CIR-DAG: cir.global external @fam_realign = #cir.const_record<{#cir.int<113> : !s8i, #cir.const_record<{#cir.int<7> : !s32i}> : !rec_PtrToIntUnion, #cir.const_array<[#cir.int<97> : !s8i, #cir.int<98> : !s8i, #cir.int<99> : !s8i]> : !cir.array<!s8i x 3>}> : !rec_FamMoves +// LLVM-DAG: @fam_realign = global <{ i8, [7 x i8], { i32, [4 x i8] }, [3 x i8] }> <{ i8 113, [7 x i8] zeroinitializer, { i32, [4 x i8] } { i32 7, [4 x i8] zeroinitializer }, [3 x i8] c"abc" }> + + typedef union vec3 { struct { double x, y, z; }; double component[3]; } vec3; -// LLVMCIR: @__const.ret_outer.__retval = {{.*}}%struct.outer { %union.needs_padding zeroinitializer, i32 1 } -// OGCG: @__const.ret_outer.o = {{.*}}{ { i32, [4 x i8] }, i32, [4 x i8] } { { i32, [4 x i8] } zeroinitializer, i32 1, [4 x i8] zeroinitializer } +// LLVMCIR-DAG: @__const.ret_outer.__retval = {{.*}}%struct.outer { %union.needs_padding zeroinitializer, i32 1 } +// OGCG-DAG: @__const.ret_outer.o = {{.*}}{ { i32, [4 x i8] }, i32, [4 x i8] } { { i32, [4 x i8] } zeroinitializer, i32 1, [4 x i8] zeroinitializer } -// CIR: cir.global "private" constant cir_private @__const.ret_outer.__retval = #cir.const_record<{#cir.zero : !rec_needs_padding, #cir.int<1> : !s32i}> : !rec_outer +// CIR-DAG: cir.global "private" constant cir_private @__const.ret_outer.__retval = #cir.const_record<{#cir.zero : !rec_needs_padding, #cir.int<1> : !s32i}> : !rec_outer // In C mode, this does do zero padding. vec3 ret_vec3() { @@ -67,3 +119,4 @@ struct outer ret_outer() { // OGCG: %[[RET:.*]] = load { i64, i32 }, ptr %[[RET_ALLOCA]] // LLVM: ret { i64, i32 } %[[RET]] } + diff --git a/clang/test/CIR/CodeGen/union-agg-init.cpp b/clang/test/CIR/CodeGen/union-agg-init.cpp index 86c94c596572c..8ce84648577e8 100644 --- a/clang/test/CIR/CodeGen/union-agg-init.cpp +++ b/clang/test/CIR/CodeGen/union-agg-init.cpp @@ -5,6 +5,19 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll // RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s +union PtrToIntUnion { int id; char *str; }; +struct Base { int b; }; +struct Derived : Base { union PtrToIntUnion u; }; +struct DerivedFromDerived : Derived { int i; }; +Derived derived = { { 1 }, { 2 } }; +// CIR: cir.global external @derived = #cir.const_record<{#cir.const_record<{#cir.int<1> : !s32i}> : !rec_Base, #cir.const_record<{#cir.int<2> : !s32i}> : !rec_PtrToIntUnion}> : !rec_Derived +// LLVMCIR: @derived = global { %struct.Base, [4 x i8], { i32, [4 x i8] } } { %struct.Base { i32 1 }, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 2, [4 x i8] zeroinitializer } } +// OGCG: @derived = global { i32, [4 x i8], { i32, [4 x i8] } } { i32 1, [4 x i8] undef, { i32, [4 x i8] } { i32 2, [4 x i8] undef } }, align 8 +DerivedFromDerived derivedFromDerived = { { { 1 }, { 2 } }, 3}; +// CIR: cir.global external @derivedFromDerived = #cir.const_record<{#cir.const_record<{#cir.const_record<{#cir.int<1> : !s32i}> : !rec_Base, #cir.const_record<{#cir.int<2> : !s32i}> : !rec_PtrToIntUnion}> : !rec_Derived, #cir.int<3> : !s32i, #cir.zero : !cir.array<!u8i x 4>}> : !rec_DerivedFromDerived +// LLVMCIR: @derivedFromDerived = global <{ { %struct.Base, [4 x i8], { i32, [4 x i8] } }, i32, [4 x i8] }> <{ { %struct.Base, [4 x i8], { i32, [4 x i8] } } { %struct.Base { i32 1 }, [4 x i8] zeroinitializer, { i32, [4 x i8] } { i32 2, [4 x i8] zeroinitializer } }, i32 3, [4 x i8] zeroinitializer }> +// OGCG: @derivedFromDerived = global { i32, [4 x i8], { i32, [4 x i8] }, i32, [4 x i8] } { i32 1, [4 x i8] undef, { i32, [4 x i8] } { i32 2, [4 x i8] undef }, i32 3, [4 x i8] undef }, align 8 + typedef union vec3 { struct { double x, y, z; }; double component[3]; @@ -63,3 +76,4 @@ extern "C" Trivial ret_trivial() { return {}; } // OGCG: %[[COERCE_DIVE:.*]] = getelementptr inbounds nuw %union.Trivial, ptr %[[RET_ALLOCA]], i32 0, i32 0 // OGCG: %[[RET:.*]] = load i32, ptr %[[COERCE_DIVE]] // LLVM: ret i32 %[[RET]] + diff --git a/clang/test/CIR/CodeGen/unions-with-zero-init.cpp b/clang/test/CIR/CodeGen/unions-with-zero-init.cpp index 992925ef85c2c..9c9cdc0165a15 100644 --- a/clang/test/CIR/CodeGen/unions-with-zero-init.cpp +++ b/clang/test/CIR/CodeGen/unions-with-zero-init.cpp @@ -39,7 +39,7 @@ // This gets promoted to a constant, so it is up here. // CIR-AFTER-DAG: cir.global "private" constant cir_private @__const._Z1fv.inner_a2 = #cir.const_record<{#cir.int<12> : !s32i}> : !rec_inner_aggregate2 -// LLVM-DAG: @__const._Z1fv.inner_a2 = private {{.*}}constant { i32, [4 x i8] } { i32 12, [4 x i8] undef } +// LLVM-DAG: @__const._Z1fv.inner_a2 = private {{.*}}constant { i32, [4 x i8] } { i32 12, [4 x i8] {{.*}} } struct S { int x; }; int S::* p = nullptr; @@ -54,7 +54,7 @@ int S::* p = nullptr; // LLVMCIR-DAG: @outer_a1 = global %union.outer_aggregate zeroinitializer // OGCG-DAG: @outer_a1 = global %union.outer_aggregate { i64 -1 } -// LLVM-DAG: @outer_a2 = global { i32, [4 x i8] } { i32 32, [4 x i8] undef } +// LLVM-DAG: @outer_a2 = global { i32, [4 x i8] } { i32 32, [4 x i8] {{.*}} } // LLVM-DAG: @outer_a3 = global %union.outer_aggregate3 { i64 -1 } struct HasPtrToMember { int S::*p; }; // not zero-initializable @@ -65,7 +65,7 @@ union U { U u{}; // CIR-DAG: cir.global external @u = #cir.zero : !rec_U // LLVMCIR-DAG: @u = global %union.U zeroinitializer -// OGCG-DAG: @u = global { i32, [4 x i8] } { i32 0, [4 x i8] undef } +// OGCG-DAG: @u = global { i32, [4 x i8] } { i32 0, [4 x i8] {{.*}} } auto use() { return u; } >From e43a1fdbd2b1180ad66c1626ce6e2e746290bc35 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Fri, 14 Aug 2026 09:23:38 -0700 Subject: [PATCH 2/2] clang-format --- clang/lib/CIR/Lowering/LoweringHelpers.cpp | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/clang/lib/CIR/Lowering/LoweringHelpers.cpp b/clang/lib/CIR/Lowering/LoweringHelpers.cpp index db6448ee0023f..3dc79b3b10d21 100644 --- a/clang/lib/CIR/Lowering/LoweringHelpers.cpp +++ b/clang/lib/CIR/Lowering/LoweringHelpers.cpp @@ -442,8 +442,8 @@ static mlir::Type adjustGlobalStructTypeForInit( for (auto [idx, member] : llvm::enumerate(initMembers)) { if (idx >= origBody.size()) break; - mlir::Type adjusted = adjustGlobalTypeForInit( - origBody[idx], member, converter, dataLayout); + mlir::Type adjusted = + adjustGlobalTypeForInit(origBody[idx], member, converter, dataLayout); unsigned adjustedAlign = dataLayout.getTypeABIAlignment(adjusted); if (adjusted != origBody[idx]) { @@ -467,8 +467,7 @@ static mlir::Type adjustGlobalStructTypeForInit( // alignment between this field and the padding. uint64_t difference = origOffset - curOffset; newBody.push_back(mlir::LLVM::LLVMArrayType::get( - mlir::IntegerType::get(structTy.getContext(), 8), - difference)); + mlir::IntegerType::get(structTy.getContext(), 8), difference)); paddingAddedIndexes.push_back(idx); curOffset = origOffset; } @@ -624,15 +623,15 @@ std::optional<mlir::Attribute> lowerConstRecordAttr( } // The lowered LLVM type may have more fields than the CIR record has members - // for a few reasons: - // 1- a union lowers to { active-member, [pad x i8]). + // for a few reasons: + // 1- a union lowers to { active-member, [pad x i8]). // 2- A struct that contains such a union can have its alignment changed too, // so it needs tail padding to fill that in. - // 3- A struct containing a union whose initializer doesn't use the highest-aligned - // field will have to prepend a bit of padding, such as struct { i32, union { - // i64, i32 } }. Typically the union gets lowered to a struct { i64 } (as i64 - // has the greatest alignment), but if the init causes it to be the i32(or any - // such smaller field) we have to prepend it with padding: + // 3- A struct containing a union whose initializer doesn't use the + // highest-aligned field will have to prepend a bit of padding, such as struct + // { i32, union { i64, i32 } }. Typically the union gets lowered to a struct + // { i64 } (as i64 has the greatest alignment), but if the init causes it to + // be the i32(or any such smaller field) we have to prepend it with padding: // struct { i32, [4 x i8], struct { i32 }} // instead of (with no init): // struct { i32, struct { i64 }} @@ -646,7 +645,7 @@ std::optional<mlir::Attribute> lowerConstRecordAttr( // causing problems. for (unsigned paddedElt : llvm::reverse(paddingAddedIndexes)) loweredMembers.insert(loweredMembers.begin() + paddedElt, - mlir::LLVM::ZeroAttr::get(constRecord.getContext())); + mlir::LLVM::ZeroAttr::get(constRecord.getContext())); // Any remaining difference will be the union/struct padding case. We don't // have a great handle/way to tell when to zero-vs-undef init, so always _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
