https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/208256
>From c5cab0df826e2dfc195377c889926e262cdad7f7 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Wed, 8 Jul 2026 09:24:56 -0700 Subject: [PATCH 01/17] [Clang] Fix x86_fp80 misalignment with pragma pack on Windows --- clang/lib/AST/RecordLayoutBuilder.cpp | 8 ++- clang/test/CodeGen/x86_fp80-alignment-win.cpp | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 clang/test/CodeGen/x86_fp80-alignment-win.cpp diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c27572bc7f50d..6df1bc37a890d 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2748,10 +2748,12 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } - // Respect pragma pack, attribute pack and declspec align - if (!MaxFieldAlignment.isZero()) + // Respect pragma pack, attribute pack and declspec align, but not for types + // that require specific alignment for correctness (e.g., x86_fp80 needs + // 16-byte alignment for movaps instructions). + if (!MaxFieldAlignment.isZero() && !ContainsX86FP80) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); - if (FD->hasAttr<PackedAttr>()) + if (FD->hasAttr<PackedAttr>() && !ContainsX86FP80) Info.Alignment = CharUnits::One(); // The alignment used to update the record's alignment excludes over-alignment // applied directly to the field; the alignment used for placement includes diff --git a/clang/test/CodeGen/x86_fp80-alignment-win.cpp b/clang/test/CodeGen/x86_fp80-alignment-win.cpp new file mode 100644 index 0000000000000..f446474c598be --- /dev/null +++ b/clang/test/CodeGen/x86_fp80-alignment-win.cpp @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -mlong-double-80 -emit-llvm -o - %s | FileCheck %s + +// Test that x86_fp80 (long double with /Qlong-double flag) maintains +// 16-byte alignment for correctness (required by movaps instructions), +// even when #pragma pack would normally reduce it. + +struct Klass { + long double a; +}; + +// Simulate std::array without including headers +template<typename T, unsigned N> +struct array { + T _Elems[N]; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_single_klass +// CHECK: %k = alloca %struct.Klass, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_single_klass() { + Klass k; + k.a = 0.0L; +} + +// CHECK-LABEL: define {{.*}} @{{.*}}test_struct_array +// CHECK: %matrix = alloca %struct.array, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_struct_array() { + array<Klass, 16> matrix; + for (int i = 0; i < 16; i++) + matrix._Elems[i].a = 0.0L; +} + +// CHECK-LABEL: define {{.*}} @{{.*}}test_direct_array +// CHECK: %arr = alloca [16 x %struct.Klass], align 16 +void test_direct_array() { + Klass arr[16]; + for (int i = 0; i < 16; i++) + arr[i].a = 0.0L; +} + +// Test with explicit pragma pack(8) - should still maintain 16-byte alignment +#pragma pack(push, 8) +struct PackedKlass { + long double b; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_explicit_pack +// CHECK: %pk = alloca %struct.PackedKlass, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_explicit_pack() { + PackedKlass pk; + pk.b = 0.0L; +} +#pragma pack(pop) >From 8cd7220220454e52b4c74fbfb6bcd40eb5e3e00f Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Fri, 10 Jul 2026 07:26:10 -0700 Subject: [PATCH 02/17] Leveraged FieldRequiredAlignment --- clang/lib/AST/RecordLayoutBuilder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 6df1bc37a890d..8b15c0a78a89f 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2751,9 +2751,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // Respect pragma pack, attribute pack and declspec align, but not for types // that require specific alignment for correctness (e.g., x86_fp80 needs // 16-byte alignment for movaps instructions). - if (!MaxFieldAlignment.isZero() && !ContainsX86FP80) + if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); - if (FD->hasAttr<PackedAttr>() && !ContainsX86FP80) + if (FD->hasAttr<PackedAttr>()) Info.Alignment = CharUnits::One(); // The alignment used to update the record's alignment excludes over-alignment // applied directly to the field; the alignment used for placement includes >From 8b2032e0a989cd4c39705a87ada08de89b98e84e Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Tue, 14 Jul 2026 08:24:02 -0700 Subject: [PATCH 03/17] Added vector type support and addressed review comments --- clang/include/clang/AST/ASTContext.h | 3 + clang/lib/AST/ASTContext.cpp | 28 +++- clang/lib/AST/RecordLayoutBuilder.cpp | 96 ++++++++++++- .../x86_fp80-alignment-pragma-pack.cpp | 136 ++++++++++++++++++ clang/test/CodeGen/x86_fp80-alignment-win.cpp | 58 -------- 5 files changed, 253 insertions(+), 68 deletions(-) create mode 100644 clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp delete mode 100644 clang/test/CodeGen/x86_fp80-alignment-win.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index bd4dd2ad06aac..97d885f7af46f 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -183,6 +183,9 @@ enum class AlignRequirementKind { /// The alignment comes from an alignment attribute on a enum type. RequiredByEnum, + + /// The alignment is required by the ABI for correctness. + RequiredByABI, }; struct TypeInfo { diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 5539a39d6edaf..57c81c83dac03 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2349,6 +2349,15 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = Target->getLongDoubleWidth(); Align = Target->getLongDoubleAlign(); } + // On Windows targets, x86_fp80 requires 16-byte alignment for ABI + // correctness (movaps instructions will fault on misaligned addresses). + // Mark this as an ABI requirement that must not be reduced by #pragma + // pack. GCC preserves such alignment on other targets, but Clang + // historically has not; changing this would break existing Clang ABI on + // non-Windows platforms. + if (Target->getTriple().isOSWindows() && + &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) + AlignRequirement = AlignRequirementKind::RequiredByABI; break; case BuiltinType::Float128: if (Target->hasFloat128Type() || !getLangOpts().OpenMP || @@ -2546,9 +2555,22 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { const ASTRecordLayout &Layout = getASTRecordLayout(RD); Width = toBits(Layout.getSize()); Align = toBits(Layout.getAlignment()); - AlignRequirement = RD->hasAttr<AlignedAttr>() - ? AlignRequirementKind::RequiredByRecord - : AlignRequirementKind::None; + // Check if the record has an aligned attribute, or if it contains + // fields with ABI-required alignment (e.g., x86_fp80). + if (RD->hasAttr<AlignedAttr>()) { + AlignRequirement = AlignRequirementKind::RequiredByRecord; + } else { + // Check if any field has RequiredByABI alignment requirement. + // If so, propagate it to the record. + AlignRequirement = AlignRequirementKind::None; + for (const auto *Field : RD->fields()) { + TypeInfo FI = getTypeInfo(Field->getType().getTypePtr()); + if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI) { + AlignRequirement = AlignRequirementKind::RequiredByABI; + break; + } + } + } break; } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 8b15c0a78a89f..c0b183a2ce6a6 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -559,6 +559,61 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects( typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy; +/// Helper for RequiresVectorAlignment - recursively checks types. +static bool CheckTypeForRequiredVectorAlignment(const ASTContext &Context, + QualType Ty) { + if (const auto *VT = Ty->getAs<VectorType>()) { + uint64_t VecWidth = Context.getTypeSize(VT); + return VT->getVectorKind() == VectorKind::Generic && + (VecWidth == 128 || VecWidth == 256) && + VecWidth == Context.getTypeAlign(VT); + } + if (const auto *AT = Ty->getAsArrayTypeUnsafe()) + return CheckTypeForRequiredVectorAlignment(Context, AT->getElementType()); + + if (const auto *RT = Ty->getAs<RecordType>()) { + for (const auto *Field : RT->getDecl()->fields()) + if (CheckTypeForRequiredVectorAlignment(Context, Field->getType())) + return true; + } + return false; +} + +/// Check if a type (or any type it contains) is a standard SIMD vector requiring +/// alignment preservation on Windows. This includes direct vectors, arrays of +/// vectors, and structs containing vectors. +static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { + if (!Context.getTargetInfo().getTriple().isOSWindows()) + return false; + return CheckTypeForRequiredVectorAlignment(Context, Ty); +} + +/// Check if we should prevent MaxFieldAlignment from reducing this field's +/// alignment. Returns true if the field has ABI-required alignment or contains +/// vectors requiring alignment, UNLESS the struct has explicit packed attribute. +static bool ShouldPreserveFieldAlignment(const ASTContext &Context, + const FieldDecl *FD, + AlignRequirementKind AlignReq, + bool StructHasPackedAttr) { + if (AlignReq == AlignRequirementKind::RequiredByABI) + return true; + if (!StructHasPackedAttr && RequiresVectorAlignment(Context, FD->getType())) + return true; + return false; +} + +/// Check if a record contains any fields with vectors requiring alignment. +/// Returns false if the record has explicit __attribute__((packed)). +static bool RecordContainsVectorRequiringAlignment(const ASTContext &Context, + const RecordDecl *RD) { + if (RD->hasAttr<PackedAttr>()) + return false; + for (const auto *Field : RD->fields()) + if (RequiresVectorAlignment(Context, Field->getType())) + return true; + return false; +} + class ItaniumRecordLayoutBuilder { protected: // FIXME: Remove this and make the appropriate fields public. @@ -2029,13 +2084,20 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, UnpackedFieldAlign = std::max(UnpackedFieldAlign, MaxAlignmentInChars); // The maximum field alignment overrides the aligned attribute. - if (!MaxFieldAlignment.isZero()) { + // However, do not reduce alignment for ABI-required alignments (e.g., + // x86_fp80, vector types) which must be preserved for correctness. + // On Windows, check if the field type is a vector with standard SIMD + // alignment (16 or 32 bytes with size == alignment) - these need their + // alignment preserved under #pragma pack. However, honor explicit + // __attribute__((packed)) on the struct (Packed=true means the struct + // has the packed attribute, not the field). + if (!MaxFieldAlignment.isZero() && + !ShouldPreserveFieldAlignment(Context, D, AlignRequirement, Packed)) { PackedFieldAlign = std::min(PackedFieldAlign, MaxFieldAlignment); PreferredAlign = std::min(PreferredAlign, MaxFieldAlignment); UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment); } - if (!FieldPacked) FieldAlign = UnpackedFieldAlign; if (DefaultsToAIXPowerAlignment) @@ -2748,10 +2810,25 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } - // Respect pragma pack, attribute pack and declspec align, but not for types - // that require specific alignment for correctness (e.g., x86_fp80 needs - // 16-byte alignment for movaps instructions). - if (!MaxFieldAlignment.isZero()) + // Check if this is a vector type (or contains vectors) requiring alignment + // preservation on Windows. This includes direct vectors, arrays of vectors, + // and structs containing vectors. However, honor __attribute__((packed)) + // on the struct even for vectors (explicit intent to pack). + bool IsVectorRequiringAlignment = false; + if (const RecordDecl *RD = FD->getParent()) { + if (!RD->hasAttr<PackedAttr>()) { + IsVectorRequiringAlignment = RequiresVectorAlignment(Context, FD->getType()); + } + } + + // Respect pragma pack, attribute pack and declspec align. + // However, do not reduce alignment for ABI-required alignments (e.g., + // x86_fp80, vector types) which must be preserved for correctness. + bool StructHasPackedAttr = + FD->getParent() && FD->getParent()->hasAttr<PackedAttr>(); + if (!MaxFieldAlignment.isZero() && + !ShouldPreserveFieldAlignment(Context, FD, TInfo.AlignRequirement, + StructHasPackedAttr)) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); if (FD->hasAttr<PackedAttr>()) Info.Alignment = CharUnits::One(); @@ -3289,7 +3366,12 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { if (!RequiredAlignment.isZero()) { Alignment = std::max(Alignment, RequiredAlignment); auto RoundingAlignment = Alignment; - if (!MaxFieldAlignment.isZero()) + // Check if this struct contains vectors that require ABI alignment before + // allowing MaxFieldAlignment (from #pragma pack) to reduce the overall + // struct alignment. However, if the struct has __attribute__((packed)), + // honor that explicit request even for vectors. + if (!MaxFieldAlignment.isZero() && + !RecordContainsVectorRequiringAlignment(Context, RD)) RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment); RoundingAlignment = std::max(RoundingAlignment, RequiredAlignment); Size = Size.alignTo(RoundingAlignment); diff --git a/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp b/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp new file mode 100644 index 0000000000000..accc79dde04f0 --- /dev/null +++ b/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp @@ -0,0 +1,136 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu -emit-llvm -o - %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-FP80 + +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-VEC + +// Test that x86_fp80 (long double) and vector types maintain their required +// alignment for correctness (movaps/movapd instructions will fault on misaligned +// addresses), even when #pragma pack(8) would normally reduce it. This issue +// affects Windows targets where #pragma pack is commonly used. +// Note: GCC on Linux preserves such alignment even with #pragma pack, so this fix +// is Windows-specific to avoid breaking existing Clang ABI on other platforms. +// Note: We use windows-gnu (MinGW) for x86_fp80 tests because windows-msvc doesn't +// support 80-bit long double. + +typedef float v4f32 __attribute__((vector_size(16))); +typedef float v8f32 __attribute__((vector_size(32))); + +struct Klass { + long double a; +}; + +struct VectorKlass { + v4f32 v; +}; + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_single_klass +// CHECK-FP80: %k = alloca %struct.Klass, align 16 +// CHECK-FP80: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_single_klass() { + Klass k; + k.a = 0.0L; +} + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_vector_klass +// CHECK-VEC: %v = alloca %struct.VectorKlass, align 16 +void test_vector_klass() { + VectorKlass v; + v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// Test with pragma pack(8) - should STILL maintain required alignment +// This is the key test case for the bug fix +#pragma pack(push, 8) + +struct PackedKlass { + long double b; +}; + +struct PackedVectorKlass { + v4f32 v; +}; + +struct PackedLargeVectorKlass { + v8f32 v; +}; + +// Simulate std::array without including headers +template<typename T, unsigned N> +struct array { + T _Elems[N]; +}; + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_explicit_pack +// CHECK-FP80: %pk = alloca %struct.PackedKlass, align 16 +// CHECK-FP80: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_explicit_pack() { + PackedKlass pk; + pk.b = 0.0L; +} + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_packed_vector +// CHECK-VEC: %pv = alloca %struct.PackedVectorKlass, align 16 +void test_packed_vector() { + PackedVectorKlass pv; + pv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_packed_large_vector +// CHECK-VEC: %plv = alloca %struct.PackedLargeVectorKlass, align 32 +void test_packed_large_vector() { + PackedLargeVectorKlass plv; + plv.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}; +} + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_struct_array_packed +// CHECK-FP80: %matrix = alloca %struct.array, align 16 +void test_struct_array_packed() { + array<PackedKlass, 16> matrix; + for (int i = 0; i < 16; i++) + matrix._Elems[i].b = 0.0L; +} + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_direct_array_packed +// CHECK-FP80: %arr = alloca [16 x %struct.PackedKlass], align 16 +void test_direct_array_packed() { + PackedKlass arr[16]; + for (int i = 0; i < 16; i++) + arr[i].b = 0.0L; +} + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_vector_array_packed +// CHECK-VEC: %varr = alloca %struct.array{{.*}}, align 16 +void test_vector_array_packed() { + array<PackedVectorKlass, 4> varr; + for (int i = 0; i < 4; i++) + varr._Elems[i].v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(pop) + +// Test that __attribute__((packed)) on the struct reduces vector alignment. +// This is needed for unaligned load/store intrinsics like _mm_loadu_ps. +struct __attribute__((packed)) ExplicitlyPackedVector { + v4f32 v; +}; + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_explicitly_packed_vector +// CHECK-VEC: %epv = alloca %struct.ExplicitlyPackedVector, align 1 +void test_explicitly_packed_vector() { + ExplicitlyPackedVector epv; + epv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(push, 8) +struct FieldPackedVector { + v4f32 v __attribute__((packed)); +}; +#pragma pack(pop) + +// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_field_packed_vector +// CHECK-VEC: %fpv = alloca %struct.FieldPackedVector, align 1 +void test_field_packed_vector() { + FieldPackedVector fpv; + fpv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} diff --git a/clang/test/CodeGen/x86_fp80-alignment-win.cpp b/clang/test/CodeGen/x86_fp80-alignment-win.cpp deleted file mode 100644 index f446474c598be..0000000000000 --- a/clang/test/CodeGen/x86_fp80-alignment-win.cpp +++ /dev/null @@ -1,58 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -mlong-double-80 -emit-llvm -o - %s | FileCheck %s - -// Test that x86_fp80 (long double with /Qlong-double flag) maintains -// 16-byte alignment for correctness (required by movaps instructions), -// even when #pragma pack would normally reduce it. - -struct Klass { - long double a; -}; - -// Simulate std::array without including headers -template<typename T, unsigned N> -struct array { - T _Elems[N]; -}; - -// CHECK-LABEL: define {{.*}} @{{.*}}test_single_klass -// CHECK: %k = alloca %struct.Klass, align 16 -// CHECK-NOT: align 8 -// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 -void test_single_klass() { - Klass k; - k.a = 0.0L; -} - -// CHECK-LABEL: define {{.*}} @{{.*}}test_struct_array -// CHECK: %matrix = alloca %struct.array, align 16 -// CHECK-NOT: align 8 -// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 -void test_struct_array() { - array<Klass, 16> matrix; - for (int i = 0; i < 16; i++) - matrix._Elems[i].a = 0.0L; -} - -// CHECK-LABEL: define {{.*}} @{{.*}}test_direct_array -// CHECK: %arr = alloca [16 x %struct.Klass], align 16 -void test_direct_array() { - Klass arr[16]; - for (int i = 0; i < 16; i++) - arr[i].a = 0.0L; -} - -// Test with explicit pragma pack(8) - should still maintain 16-byte alignment -#pragma pack(push, 8) -struct PackedKlass { - long double b; -}; - -// CHECK-LABEL: define {{.*}} @{{.*}}test_explicit_pack -// CHECK: %pk = alloca %struct.PackedKlass, align 16 -// CHECK-NOT: align 8 -// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 -void test_explicit_pack() { - PackedKlass pk; - pk.b = 0.0L; -} -#pragma pack(pop) >From effa4d8456870a739f00f1cd091545d9419469a8 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Tue, 14 Jul 2026 09:15:14 -0700 Subject: [PATCH 04/17] Fix format --- clang/lib/AST/RecordLayoutBuilder.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c0b183a2ce6a6..cd0f48071a020 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -579,9 +579,9 @@ static bool CheckTypeForRequiredVectorAlignment(const ASTContext &Context, return false; } -/// Check if a type (or any type it contains) is a standard SIMD vector requiring -/// alignment preservation on Windows. This includes direct vectors, arrays of -/// vectors, and structs containing vectors. +/// Check if a type (or any type it contains) is a standard SIMD vector +/// requiring alignment preservation on Windows. This includes direct vectors, +/// arrays of vectors, and structs containing vectors. static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { if (!Context.getTargetInfo().getTriple().isOSWindows()) return false; @@ -590,7 +590,8 @@ static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { /// Check if we should prevent MaxFieldAlignment from reducing this field's /// alignment. Returns true if the field has ABI-required alignment or contains -/// vectors requiring alignment, UNLESS the struct has explicit packed attribute. +/// vectors requiring alignment, UNLESS the struct has explicit packed +/// attribute. static bool ShouldPreserveFieldAlignment(const ASTContext &Context, const FieldDecl *FD, AlignRequirementKind AlignReq, @@ -2817,7 +2818,8 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( bool IsVectorRequiringAlignment = false; if (const RecordDecl *RD = FD->getParent()) { if (!RD->hasAttr<PackedAttr>()) { - IsVectorRequiringAlignment = RequiresVectorAlignment(Context, FD->getType()); + IsVectorRequiringAlignment = + RequiresVectorAlignment(Context, FD->getType()); } } >From f1b6a68074cb2fd4b111b5ad5a235fb24a727964 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Tue, 14 Jul 2026 09:44:00 -0700 Subject: [PATCH 05/17] Removed obsolete code --- clang/lib/AST/RecordLayoutBuilder.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index cd0f48071a020..c0e8e30a3061a 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2811,17 +2811,6 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } - // Check if this is a vector type (or contains vectors) requiring alignment - // preservation on Windows. This includes direct vectors, arrays of vectors, - // and structs containing vectors. However, honor __attribute__((packed)) - // on the struct even for vectors (explicit intent to pack). - bool IsVectorRequiringAlignment = false; - if (const RecordDecl *RD = FD->getParent()) { - if (!RD->hasAttr<PackedAttr>()) { - IsVectorRequiringAlignment = - RequiresVectorAlignment(Context, FD->getType()); - } - } // Respect pragma pack, attribute pack and declspec align. // However, do not reduce alignment for ABI-required alignments (e.g., >From fb25906f57810c55712c2078bbdc63e2f8423ff5 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Thu, 16 Jul 2026 08:27:54 -0700 Subject: [PATCH 06/17] Limited changes to msvc and addressed code propagamtion comment --- clang/lib/AST/ASTContext.cpp | 15 +- clang/lib/AST/RecordLayoutBuilder.cpp | 6 +- .../vector-alignment-pragma-pack-msvc.cpp | 130 +++++++++++++++++ .../x86_fp80-alignment-pragma-pack.cpp | 136 ------------------ 4 files changed, 143 insertions(+), 144 deletions(-) create mode 100644 clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp delete mode 100644 clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 57c81c83dac03..78ab49cadc882 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2349,13 +2349,15 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = Target->getLongDoubleWidth(); Align = Target->getLongDoubleAlign(); } - // On Windows targets, x86_fp80 requires 16-byte alignment for ABI + // On Windows MSVC targets, x86_fp80 requires 16-byte alignment for ABI // correctness (movaps instructions will fault on misaligned addresses). // Mark this as an ABI requirement that must not be reduced by #pragma - // pack. GCC preserves such alignment on other targets, but Clang - // historically has not; changing this would break existing Clang ABI on - // non-Windows platforms. + // pack. This is MSVC-specific; MinGW has different layout rules. GCC + // preserves such alignment on other targets, but Clang historically has + // not; changing this would break existing Clang ABI on non-Windows + // platforms. if (Target->getTriple().isOSWindows() && + Target->getTriple().isWindowsMSVCEnvironment() && &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) AlignRequirement = AlignRequirementKind::RequiredByABI; break; @@ -2556,7 +2558,7 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = toBits(Layout.getSize()); Align = toBits(Layout.getAlignment()); // Check if the record has an aligned attribute, or if it contains - // fields with ABI-required alignment (e.g., x86_fp80). + // fields with ABI-required alignment (e.g., vectors on MSVC). if (RD->hasAttr<AlignedAttr>()) { AlignRequirement = AlignRequirementKind::RequiredByRecord; } else { @@ -2565,7 +2567,8 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { AlignRequirement = AlignRequirementKind::None; for (const auto *Field : RD->fields()) { TypeInfo FI = getTypeInfo(Field->getType().getTypePtr()); - if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI) { + if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI || + FI.AlignRequirement == AlignRequirementKind::RequiredByRecord) { AlignRequirement = AlignRequirementKind::RequiredByABI; break; } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c0e8e30a3061a..58ce1353de4ea 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -583,7 +583,8 @@ static bool CheckTypeForRequiredVectorAlignment(const ASTContext &Context, /// requiring alignment preservation on Windows. This includes direct vectors, /// arrays of vectors, and structs containing vectors. static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { - if (!Context.getTargetInfo().getTriple().isOSWindows()) + if (!Context.getTargetInfo().getTriple().isOSWindows() || + !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) return false; return CheckTypeForRequiredVectorAlignment(Context, Ty); } @@ -596,7 +597,8 @@ static bool ShouldPreserveFieldAlignment(const ASTContext &Context, const FieldDecl *FD, AlignRequirementKind AlignReq, bool StructHasPackedAttr) { - if (AlignReq == AlignRequirementKind::RequiredByABI) + if (AlignReq == AlignRequirementKind::RequiredByABI || + AlignReq == AlignRequirementKind::RequiredByRecord) return true; if (!StructHasPackedAttr && RequiresVectorAlignment(Context, FD->getType())) return true; diff --git a/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp new file mode 100644 index 0000000000000..d185236f2a500 --- /dev/null +++ b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp @@ -0,0 +1,130 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s \ +// RUN: | FileCheck %s + +// Test that vector types maintain their required alignment for correctness +// (movaps/movapd instructions will fault on misaligned addresses), even when +// #pragma pack(8) would normally reduce it. This issue affects Windows MSVC +// targets where #pragma pack is commonly used (e.g., MSVC STL). + +typedef float v4f32 __attribute__((vector_size(16))); +typedef float v8f32 __attribute__((vector_size(32))); + +struct VectorKlass { + v4f32 v; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_vector_klass +// CHECK: %v = alloca %struct.VectorKlass, align 16 +void test_vector_klass() { + VectorKlass v; + v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(push, 8) +struct PackedVectorKlass { + v4f32 v; +}; + +struct PackedLargeVectorKlass { + v8f32 v; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_packed_vector +// CHECK: %pv = alloca %struct.PackedVectorKlass, align 16 +void test_packed_vector() { + PackedVectorKlass pv; + pv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// CHECK-LABEL: define {{.*}} @{{.*}}test_packed_large_vector +// CHECK: %plv = alloca %struct.PackedLargeVectorKlass, align 32 +void test_packed_large_vector() { + PackedLargeVectorKlass plv; + plv.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}; +} + +struct InnerWithVector { + v4f32 v; +}; + +struct OuterWithVector { + InnerWithVector inner; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_nested_vector +// CHECK: %outer = alloca %struct.OuterWithVector, align 16 +void test_nested_vector() { + OuterWithVector outer; + outer.inner.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// Test array of structs containing vectors +template<typename T, unsigned N> +struct array { + T _Elems[N]; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_vector_array_packed +// CHECK: %varr = alloca %struct.array, align 16 +void test_vector_array_packed() { + array<PackedVectorKlass, 4> varr; + for (int i = 0; i < 4; i++) + varr._Elems[i].v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} +#pragma pack(pop) + +struct __attribute__((packed)) ExplicitlyPackedVector { + v4f32 v; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_explicitly_packed_vector +// CHECK: %epv = alloca %struct.ExplicitlyPackedVector, align 1 +void test_explicitly_packed_vector() { + ExplicitlyPackedVector epv; + epv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(push, 8) +struct FieldPackedVector { + v4f32 v __attribute__((packed)); +}; +#pragma pack(pop) + +// CHECK-LABEL: define {{.*}} @{{.*}}test_field_packed_vector +// CHECK: %fpv = alloca %struct.FieldPackedVector, align 1 +void test_field_packed_vector() { + FieldPackedVector fpv; + fpv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(push, 8) +struct alignas(16) ExplicitAlignedInner { + long double x; +}; + +struct OuterWithExplicitAligned { + ExplicitAlignedInner inner; +}; + +struct ImplicitAlignedInner { + long double x; +}; + +struct OuterWithImplicitAligned { + ImplicitAlignedInner inner; +}; +#pragma pack(pop) + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_explicit_aligned_nested +// CHECK-FP80: %outer = alloca %struct.OuterWithExplicitAligned, align 16 +void test_explicit_aligned_nested() { + OuterWithExplicitAligned outer; + outer.inner.x = 0.0L; +} + +// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_implicit_aligned_nested +// CHECK-FP80: %outer = alloca %struct.OuterWithImplicitAligned, align 16 +void test_implicit_aligned_nested() { + OuterWithImplicitAligned outer; + outer.inner.x = 0.0L; +} diff --git a/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp b/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp deleted file mode 100644 index accc79dde04f0..0000000000000 --- a/clang/test/CodeGen/x86_fp80-alignment-pragma-pack.cpp +++ /dev/null @@ -1,136 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu -emit-llvm -o - %s \ -// RUN: | FileCheck %s --check-prefix=CHECK-FP80 - -// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s \ -// RUN: | FileCheck %s --check-prefix=CHECK-VEC - -// Test that x86_fp80 (long double) and vector types maintain their required -// alignment for correctness (movaps/movapd instructions will fault on misaligned -// addresses), even when #pragma pack(8) would normally reduce it. This issue -// affects Windows targets where #pragma pack is commonly used. -// Note: GCC on Linux preserves such alignment even with #pragma pack, so this fix -// is Windows-specific to avoid breaking existing Clang ABI on other platforms. -// Note: We use windows-gnu (MinGW) for x86_fp80 tests because windows-msvc doesn't -// support 80-bit long double. - -typedef float v4f32 __attribute__((vector_size(16))); -typedef float v8f32 __attribute__((vector_size(32))); - -struct Klass { - long double a; -}; - -struct VectorKlass { - v4f32 v; -}; - -// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_single_klass -// CHECK-FP80: %k = alloca %struct.Klass, align 16 -// CHECK-FP80: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 -void test_single_klass() { - Klass k; - k.a = 0.0L; -} - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_vector_klass -// CHECK-VEC: %v = alloca %struct.VectorKlass, align 16 -void test_vector_klass() { - VectorKlass v; - v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; -} - -// Test with pragma pack(8) - should STILL maintain required alignment -// This is the key test case for the bug fix -#pragma pack(push, 8) - -struct PackedKlass { - long double b; -}; - -struct PackedVectorKlass { - v4f32 v; -}; - -struct PackedLargeVectorKlass { - v8f32 v; -}; - -// Simulate std::array without including headers -template<typename T, unsigned N> -struct array { - T _Elems[N]; -}; - -// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_explicit_pack -// CHECK-FP80: %pk = alloca %struct.PackedKlass, align 16 -// CHECK-FP80: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 -void test_explicit_pack() { - PackedKlass pk; - pk.b = 0.0L; -} - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_packed_vector -// CHECK-VEC: %pv = alloca %struct.PackedVectorKlass, align 16 -void test_packed_vector() { - PackedVectorKlass pv; - pv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; -} - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_packed_large_vector -// CHECK-VEC: %plv = alloca %struct.PackedLargeVectorKlass, align 32 -void test_packed_large_vector() { - PackedLargeVectorKlass plv; - plv.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}; -} - -// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_struct_array_packed -// CHECK-FP80: %matrix = alloca %struct.array, align 16 -void test_struct_array_packed() { - array<PackedKlass, 16> matrix; - for (int i = 0; i < 16; i++) - matrix._Elems[i].b = 0.0L; -} - -// CHECK-FP80-LABEL: define {{.*}} @{{.*}}test_direct_array_packed -// CHECK-FP80: %arr = alloca [16 x %struct.PackedKlass], align 16 -void test_direct_array_packed() { - PackedKlass arr[16]; - for (int i = 0; i < 16; i++) - arr[i].b = 0.0L; -} - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_vector_array_packed -// CHECK-VEC: %varr = alloca %struct.array{{.*}}, align 16 -void test_vector_array_packed() { - array<PackedVectorKlass, 4> varr; - for (int i = 0; i < 4; i++) - varr._Elems[i].v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; -} - -#pragma pack(pop) - -// Test that __attribute__((packed)) on the struct reduces vector alignment. -// This is needed for unaligned load/store intrinsics like _mm_loadu_ps. -struct __attribute__((packed)) ExplicitlyPackedVector { - v4f32 v; -}; - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_explicitly_packed_vector -// CHECK-VEC: %epv = alloca %struct.ExplicitlyPackedVector, align 1 -void test_explicitly_packed_vector() { - ExplicitlyPackedVector epv; - epv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; -} - -#pragma pack(push, 8) -struct FieldPackedVector { - v4f32 v __attribute__((packed)); -}; -#pragma pack(pop) - -// CHECK-VEC-LABEL: define {{.*}} @{{.*}}test_field_packed_vector -// CHECK-VEC: %fpv = alloca %struct.FieldPackedVector, align 1 -void test_field_packed_vector() { - FieldPackedVector fpv; - fpv.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; -} >From c694a6d70229a37f888393d95c2e8feaa6a14383 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Thu, 16 Jul 2026 08:34:33 -0700 Subject: [PATCH 07/17] Fixed format --- clang/lib/AST/RecordLayoutBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 58ce1353de4ea..e136495ea83e6 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -583,7 +583,7 @@ static bool CheckTypeForRequiredVectorAlignment(const ASTContext &Context, /// requiring alignment preservation on Windows. This includes direct vectors, /// arrays of vectors, and structs containing vectors. static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { - if (!Context.getTargetInfo().getTriple().isOSWindows() || + if (!Context.getTargetInfo().getTriple().isOSWindows() || !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) return false; return CheckTypeForRequiredVectorAlignment(Context, Ty); >From c432d794f9b0fd1b3a952af8147421fb0ed2b5e9 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Thu, 23 Jul 2026 10:41:45 -0700 Subject: [PATCH 08/17] Addressed review comments --- clang/include/clang/AST/ASTContext.h | 5 ++ clang/lib/AST/ASTContext.cpp | 26 +++++++++++ clang/lib/AST/RecordLayoutBuilder.cpp | 66 +++++++++------------------ 3 files changed, 53 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 97d885f7af46f..1e51985dfb330 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -2763,6 +2763,11 @@ class ASTContext : public RefCountedBase<ASTContext> { TypeInfo getTypeInfo(const Type *T) const; TypeInfo getTypeInfo(QualType T) const { return getTypeInfo(T.getTypePtr()); } + /// Check if a type requires natural alignment preservation under #pragma pack + /// (but not explicit __attribute__((packed))). This includes x86_fp80 on + /// Windows MSVC and standard SIMD vectors (__m128, __m256). + bool typeRequiresPreserveAlignUnderPragmaPack(QualType T) const; + /// Get default simd alignment of the specified complete type in bits. unsigned getOpenMPDefaultSimdAlign(QualType T) const; diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 78ab49cadc882..5976487fc2990 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2101,6 +2101,32 @@ TypeInfo ASTContext::getTypeInfo(const Type *T) const { return TI; } +bool ASTContext::typeRequiresPreserveAlignUnderPragmaPack(QualType T) const { + T = T.getCanonicalType(); + const llvm::Triple &Triple = Target->getTriple(); + if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { + if (const auto *BT = T->getAs<BuiltinType>()) { + if (BT->getKind() == BuiltinType::LongDouble && + &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) + return true; + } + if (const auto *VT = T->getAs<VectorType>()) { + uint64_t VecWidth = getTypeSize(VT); + return VT->getVectorKind() == VectorKind::Generic && + (VecWidth == 128 || VecWidth == 256) && + VecWidth == getTypeAlign(VT); + } + } + if (const auto *AT = T->getAsArrayTypeUnsafe()) + return typeRequiresPreserveAlignUnderPragmaPack(AT->getElementType()); + if (const auto *RT = T->getAs<RecordType>()) { + for (const auto *Field : RT->getDecl()->fields()) + if (typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) + return true; + } + return false; +} + /// getTypeInfoImpl - Return the size of the specified type, in bits. This /// method does not work on incomplete types. /// diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index e136495ea83e6..23754c87faa3d 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -559,40 +559,10 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects( typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy; -/// Helper for RequiresVectorAlignment - recursively checks types. -static bool CheckTypeForRequiredVectorAlignment(const ASTContext &Context, - QualType Ty) { - if (const auto *VT = Ty->getAs<VectorType>()) { - uint64_t VecWidth = Context.getTypeSize(VT); - return VT->getVectorKind() == VectorKind::Generic && - (VecWidth == 128 || VecWidth == 256) && - VecWidth == Context.getTypeAlign(VT); - } - if (const auto *AT = Ty->getAsArrayTypeUnsafe()) - return CheckTypeForRequiredVectorAlignment(Context, AT->getElementType()); - - if (const auto *RT = Ty->getAs<RecordType>()) { - for (const auto *Field : RT->getDecl()->fields()) - if (CheckTypeForRequiredVectorAlignment(Context, Field->getType())) - return true; - } - return false; -} - -/// Check if a type (or any type it contains) is a standard SIMD vector -/// requiring alignment preservation on Windows. This includes direct vectors, -/// arrays of vectors, and structs containing vectors. -static bool RequiresVectorAlignment(const ASTContext &Context, QualType Ty) { - if (!Context.getTargetInfo().getTriple().isOSWindows() || - !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) - return false; - return CheckTypeForRequiredVectorAlignment(Context, Ty); -} - /// Check if we should prevent MaxFieldAlignment from reducing this field's -/// alignment. Returns true if the field has ABI-required alignment or contains -/// vectors requiring alignment, UNLESS the struct has explicit packed -/// attribute. +/// alignment. Returns true if the field has ABI-required alignment +/// (e.g., x86_fp80, SIMD vectors on Windows), unless the struct has explicit +/// __attribute__((packed)). static bool ShouldPreserveFieldAlignment(const ASTContext &Context, const FieldDecl *FD, AlignRequirementKind AlignReq, @@ -600,20 +570,27 @@ static bool ShouldPreserveFieldAlignment(const ASTContext &Context, if (AlignReq == AlignRequirementKind::RequiredByABI || AlignReq == AlignRequirementKind::RequiredByRecord) return true; - if (!StructHasPackedAttr && RequiresVectorAlignment(Context, FD->getType())) + + // Check if type requires alignment preservation under #pragma pack + // (but respect explicit __attribute__((packed))). + if (!StructHasPackedAttr && + Context.typeRequiresPreserveAlignUnderPragmaPack(FD->getType())) return true; return false; } -/// Check if a record contains any fields with vectors requiring alignment. +/// Check if a record contains any fields requiring alignment preservation. /// Returns false if the record has explicit __attribute__((packed)). -static bool RecordContainsVectorRequiringAlignment(const ASTContext &Context, - const RecordDecl *RD) { +static bool RecordContainsAlignPreservingFields(const ASTContext &Context, + const RecordDecl *RD) { if (RD->hasAttr<PackedAttr>()) return false; - for (const auto *Field : RD->fields()) - if (RequiresVectorAlignment(Context, Field->getType())) + for (const auto *Field : RD->fields()) { + TypeInfo TI = Context.getTypeInfo(Field->getType()); + if (TI.isAlignRequired() || + Context.typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) return true; + } return false; } @@ -3359,12 +3336,13 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { if (!RequiredAlignment.isZero()) { Alignment = std::max(Alignment, RequiredAlignment); auto RoundingAlignment = Alignment; - // Check if this struct contains vectors that require ABI alignment before - // allowing MaxFieldAlignment (from #pragma pack) to reduce the overall - // struct alignment. However, if the struct has __attribute__((packed)), - // honor that explicit request even for vectors. + + // Check if this struct contains fields requiring alignment preservation + // (x86_fp80, vectors) before allowing MaxFieldAlignment (from #pragma pack) + // to reduce the overall struct alignment. However, if the struct has + // __attribute__((packed)), honor that explicit request. if (!MaxFieldAlignment.isZero() && - !RecordContainsVectorRequiringAlignment(Context, RD)) + !RecordContainsAlignPreservingFields(Context, RD)) RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment); RoundingAlignment = std::max(RoundingAlignment, RequiredAlignment); Size = Size.alignTo(RoundingAlignment); >From 59724a7a8299e2edc26925373f8a78a1e847f16b Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Fri, 24 Jul 2026 06:17:10 -0700 Subject: [PATCH 09/17] Make x86_fp80 abd vector behave the same way --- clang/lib/AST/ASTContext.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 5976487fc2990..49f04561e92ff 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2375,17 +2375,6 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = Target->getLongDoubleWidth(); Align = Target->getLongDoubleAlign(); } - // On Windows MSVC targets, x86_fp80 requires 16-byte alignment for ABI - // correctness (movaps instructions will fault on misaligned addresses). - // Mark this as an ABI requirement that must not be reduced by #pragma - // pack. This is MSVC-specific; MinGW has different layout rules. GCC - // preserves such alignment on other targets, but Clang historically has - // not; changing this would break existing Clang ABI on non-Windows - // platforms. - if (Target->getTriple().isOSWindows() && - Target->getTriple().isWindowsMSVCEnvironment() && - &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) - AlignRequirement = AlignRequirementKind::RequiredByABI; break; case BuiltinType::Float128: if (Target->hasFloat128Type() || !getLangOpts().OpenMP || >From e1ea79c4141b93646b64275dfbc64144acaa1a3c Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 3 Aug 2026 08:35:16 -0700 Subject: [PATCH 10/17] Addressed review comments --- clang/lib/AST/ASTContext.cpp | 47 ++++++++------- clang/lib/AST/RecordLayoutBuilder.cpp | 3 +- .../vector-alignment-pragma-pack-itanium.cpp | 57 +++++++++++++++++++ .../vector-alignment-pragma-pack-msvc.cpp | 3 + 4 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 49f04561e92ff..d6479a87fa3bf 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2102,28 +2102,22 @@ TypeInfo ASTContext::getTypeInfo(const Type *T) const { } bool ASTContext::typeRequiresPreserveAlignUnderPragmaPack(QualType T) const { - T = T.getCanonicalType(); + // Check if type has ABI-required alignment (e.g., x86_fp80). + TypeInfo TI = getTypeInfo(T.getTypePtr()); + if (TI.AlignRequirement == AlignRequirementKind::RequiredByABI || + TI.AlignRequirement == AlignRequirementKind::RequiredByRecord) + return true; + + // For vectors, check directly without setting RequiredByABI in TypeInfo + // (to avoid affecting unaligned load/store codegen). const llvm::Triple &Triple = Target->getTriple(); if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { - if (const auto *BT = T->getAs<BuiltinType>()) { - if (BT->getKind() == BuiltinType::LongDouble && - &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) - return true; - } + T = T.getCanonicalType(); if (const auto *VT = T->getAs<VectorType>()) { - uint64_t VecWidth = getTypeSize(VT); return VT->getVectorKind() == VectorKind::Generic && - (VecWidth == 128 || VecWidth == 256) && - VecWidth == getTypeAlign(VT); + TI.Width == TI.Align; } } - if (const auto *AT = T->getAsArrayTypeUnsafe()) - return typeRequiresPreserveAlignUnderPragmaPack(AT->getElementType()); - if (const auto *RT = T->getAs<RecordType>()) { - for (const auto *Field : RT->getDecl()->fields()) - if (typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) - return true; - } return false; } @@ -2573,17 +2567,17 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { Width = toBits(Layout.getSize()); Align = toBits(Layout.getAlignment()); // Check if the record has an aligned attribute, or if it contains - // fields with ABI-required alignment (e.g., vectors on MSVC). + // fields with ABI-required alignment (e.g., x86_fp80, vectors on MSVC). if (RD->hasAttr<AlignedAttr>()) { AlignRequirement = AlignRequirementKind::RequiredByRecord; } else { - // Check if any field has RequiredByABI alignment requirement. - // If so, propagate it to the record. + // Check if any field requires alignment preservation. AlignRequirement = AlignRequirementKind::None; for (const auto *Field : RD->fields()) { TypeInfo FI = getTypeInfo(Field->getType().getTypePtr()); if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI || - FI.AlignRequirement == AlignRequirementKind::RequiredByRecord) { + FI.AlignRequirement == AlignRequirementKind::RequiredByRecord || + typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) { AlignRequirement = AlignRequirementKind::RequiredByABI; break; } @@ -2703,6 +2697,19 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { break; } + // Mark types that require ABI-correct alignment preservation under pragma pack. + if (AlignRequirement == AlignRequirementKind::None) { + const llvm::Triple &Triple = Target->getTriple(); + if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { + // x86_fp80 requires 16-byte alignment for correctness with movaps. + if (const auto *BT = T->getAs<BuiltinType>()) { + if (BT->getKind() == BuiltinType::LongDouble && + &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) + AlignRequirement = AlignRequirementKind::RequiredByABI; + } + } + } + assert(llvm::isPowerOf2_32(Align) && "Alignment must be power of 2"); return TypeInfo(Width, Align, AlignRequirement); } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 23754c87faa3d..dd8a019bd7b3b 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2066,8 +2066,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, // The maximum field alignment overrides the aligned attribute. // However, do not reduce alignment for ABI-required alignments (e.g., // x86_fp80, vector types) which must be preserved for correctness. - // On Windows, check if the field type is a vector with standard SIMD - // alignment (16 or 32 bytes with size == alignment) - these need their + // On Windows MSVC, vectors where size equals alignment need their natural // alignment preserved under #pragma pack. However, honor explicit // __attribute__((packed)) on the struct (Packed=true means the struct // has the packed attribute, not the field). diff --git a/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp b/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp new file mode 100644 index 0000000000000..1b3d943f7ea9e --- /dev/null +++ b/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-GNU + +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-LINUX + +// Test that RequiredByABI alignment works on Itanium ABI (windows-gnu) targets. + +typedef float v4f32 __attribute__((vector_size(16))); +typedef float v8f32 __attribute__((vector_size(32))); + +// Without pragma pack, vectors should have natural alignment on all platforms. + +struct VectorStruct { + v4f32 v; +}; + +// CHECK-GNU-LABEL: define {{.*}} @{{.*}}test_vector_struct +// CHECK-GNU: %v = alloca %struct.VectorStruct, align 16 +// CHECK-LINUX-LABEL: define {{.*}} @{{.*}}test_vector_struct +// CHECK-LINUX: %v = alloca %struct.VectorStruct, align 16 +void test_vector_struct() { + VectorStruct v; + v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +#pragma pack(push, 8) +struct PackedVectorStruct { + v4f32 v; +}; + +struct PackedLargeVectorStruct { + v8f32 v; +}; +#pragma pack(pop) + +// On windows-gnu and Linux, #pragma pack applies normally (no special +// preservation), so vectors in packed structs get reduced alignment. +// This differs from windows-msvc where we preserve vector alignment. + +// CHECK-GNU-LABEL: define {{.*}} @{{.*}}test_packed_vector_struct +// CHECK-GNU: %v = alloca %struct.PackedVectorStruct, align 8 +// CHECK-LINUX-LABEL: define {{.*}} @{{.*}}test_packed_vector_struct +// CHECK-LINUX: %v = alloca %struct.PackedVectorStruct, align 8 +void test_packed_vector_struct() { + PackedVectorStruct v; + v.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// CHECK-GNU-LABEL: define {{.*}} @{{.*}}test_packed_large_vector_struct +// CHECK-GNU: %v = alloca %struct.PackedLargeVectorStruct, align 8 +// CHECK-LINUX-LABEL: define {{.*}} @{{.*}}test_packed_large_vector_struct +// CHECK-LINUX: %v = alloca %struct.PackedLargeVectorStruct, align 8 +void test_packed_large_vector_struct() { + PackedLargeVectorStruct v; + v.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}; +} diff --git a/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp index d185236f2a500..c6188f656a0eb 100644 --- a/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp +++ b/clang/test/CodeGen/vector-alignment-pragma-pack-msvc.cpp @@ -1,6 +1,9 @@ // RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s \ // RUN: | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -mlong-double-80 \ +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-FP80 + // Test that vector types maintain their required alignment for correctness // (movaps/movapd instructions will fault on misaligned addresses), even when // #pragma pack(8) would normally reduce it. This issue affects Windows MSVC >From be6d2c59eb1a1639c4eddeb57777b6ff68085f78 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 3 Aug 2026 09:30:58 -0700 Subject: [PATCH 11/17] Fix format --- clang/lib/AST/ASTContext.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index d6479a87fa3bf..ec4384ae17849 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2114,8 +2114,7 @@ bool ASTContext::typeRequiresPreserveAlignUnderPragmaPack(QualType T) const { if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { T = T.getCanonicalType(); if (const auto *VT = T->getAs<VectorType>()) { - return VT->getVectorKind() == VectorKind::Generic && - TI.Width == TI.Align; + return VT->getVectorKind() == VectorKind::Generic && TI.Width == TI.Align; } } return false; @@ -2697,14 +2696,16 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { break; } - // Mark types that require ABI-correct alignment preservation under pragma pack. + // Mark types that require ABI-correct alignment preservation under pragma + // pack. if (AlignRequirement == AlignRequirementKind::None) { const llvm::Triple &Triple = Target->getTriple(); if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { // x86_fp80 requires 16-byte alignment for correctness with movaps. if (const auto *BT = T->getAs<BuiltinType>()) { if (BT->getKind() == BuiltinType::LongDouble && - &Target->getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended()) + &Target->getLongDoubleFormat() == + &llvm::APFloat::x87DoubleExtended()) AlignRequirement = AlignRequirementKind::RequiredByABI; } } >From 98681a5ec8583094ee01fe94e6271f56dea33b73 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Tue, 4 Aug 2026 05:52:42 -0700 Subject: [PATCH 12/17] Added tests for nested fields --- .../vector-alignment-pragma-pack-itanium.cpp | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp b/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp index 1b3d943f7ea9e..2f314af94fdd1 100644 --- a/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp +++ b/clang/test/CodeGen/vector-alignment-pragma-pack-itanium.cpp @@ -4,6 +4,9 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s \ // RUN: | FileCheck %s --check-prefix=CHECK-LINUX +// RUN: %clang_cc1 -triple x86_64-windows-gnu -mlong-double-80 -emit-llvm -o - %s \ +// RUN: | FileCheck %s --check-prefix=CHECK-GNU-FP80 + // Test that RequiredByABI alignment works on Itanium ABI (windows-gnu) targets. typedef float v4f32 __attribute__((vector_size(16))); @@ -55,3 +58,55 @@ void test_packed_large_vector_struct() { PackedLargeVectorStruct v; v.v = (v8f32){0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}; } + +// Test nested structs with vectors under pragma pack. + +#pragma pack(push, 8) +struct InnerWithVector { + v4f32 v; +}; + +struct OuterWithVector { + InnerWithVector inner; +}; +#pragma pack(pop) + +// On non-MSVC targets, nested vectors also get reduced alignment under pragma +// pack. + +// CHECK-GNU-LABEL: define {{.*}} @{{.*}}test_nested_vector_struct +// CHECK-GNU: %outer = alloca %struct.OuterWithVector, align 8 +// CHECK-LINUX-LABEL: define {{.*}} @{{.*}}test_nested_vector_struct +// CHECK-LINUX: %outer = alloca %struct.OuterWithVector, align 8 +void test_nested_vector_struct() { + OuterWithVector outer; + outer.inner.v = (v4f32){0.0f, 0.0f, 0.0f, 0.0f}; +} + +// Test x86_fp80 (long double with -mlong-double-80) under pragma pack. +// On windows-gnu, RequiredByABI only applies to MSVC environment, so +// pragma pack reduces alignment normally. + +#pragma pack(push, 8) +struct LongDoubleStruct { + long double x; +}; + +struct NestedLongDoubleStruct { + LongDoubleStruct inner; +}; +#pragma pack(pop) + +// CHECK-GNU-FP80-LABEL: define {{.*}} @{{.*}}test_long_double_struct +// CHECK-GNU-FP80: %s = alloca %struct.LongDoubleStruct, align 8 +void test_long_double_struct() { + LongDoubleStruct s; + s.x = 0.0L; +} + +// CHECK-GNU-FP80-LABEL: define {{.*}} @{{.*}}test_nested_long_double_struct +// CHECK-GNU-FP80: %outer = alloca %struct.NestedLongDoubleStruct, align 8 +void test_nested_long_double_struct() { + NestedLongDoubleStruct outer; + outer.inner.x = 0.0L; +} >From 712a47d5f8792e80bd0d692724e006422c421af9 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Fri, 7 Aug 2026 13:52:20 -0700 Subject: [PATCH 13/17] Use implicit aligned attributes --- clang/include/clang/AST/ASTContext.h | 8 ---- clang/lib/AST/ASTContext.cpp | 54 ++--------------------- clang/lib/AST/RecordLayoutBuilder.cpp | 62 ++------------------------- clang/lib/Sema/SemaDecl.cpp | 40 +++++++++++++++-- 4 files changed, 44 insertions(+), 120 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 1e51985dfb330..bd4dd2ad06aac 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -183,9 +183,6 @@ enum class AlignRequirementKind { /// The alignment comes from an alignment attribute on a enum type. RequiredByEnum, - - /// The alignment is required by the ABI for correctness. - RequiredByABI, }; struct TypeInfo { @@ -2763,11 +2760,6 @@ class ASTContext : public RefCountedBase<ASTContext> { TypeInfo getTypeInfo(const Type *T) const; TypeInfo getTypeInfo(QualType T) const { return getTypeInfo(T.getTypePtr()); } - /// Check if a type requires natural alignment preservation under #pragma pack - /// (but not explicit __attribute__((packed))). This includes x86_fp80 on - /// Windows MSVC and standard SIMD vectors (__m128, __m256). - bool typeRequiresPreserveAlignUnderPragmaPack(QualType T) const; - /// Get default simd alignment of the specified complete type in bits. unsigned getOpenMPDefaultSimdAlign(QualType T) const; diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index ec4384ae17849..5539a39d6edaf 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2101,25 +2101,6 @@ TypeInfo ASTContext::getTypeInfo(const Type *T) const { return TI; } -bool ASTContext::typeRequiresPreserveAlignUnderPragmaPack(QualType T) const { - // Check if type has ABI-required alignment (e.g., x86_fp80). - TypeInfo TI = getTypeInfo(T.getTypePtr()); - if (TI.AlignRequirement == AlignRequirementKind::RequiredByABI || - TI.AlignRequirement == AlignRequirementKind::RequiredByRecord) - return true; - - // For vectors, check directly without setting RequiredByABI in TypeInfo - // (to avoid affecting unaligned load/store codegen). - const llvm::Triple &Triple = Target->getTriple(); - if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { - T = T.getCanonicalType(); - if (const auto *VT = T->getAs<VectorType>()) { - return VT->getVectorKind() == VectorKind::Generic && TI.Width == TI.Align; - } - } - return false; -} - /// getTypeInfoImpl - Return the size of the specified type, in bits. This /// method does not work on incomplete types. /// @@ -2565,23 +2546,9 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { const ASTRecordLayout &Layout = getASTRecordLayout(RD); Width = toBits(Layout.getSize()); Align = toBits(Layout.getAlignment()); - // Check if the record has an aligned attribute, or if it contains - // fields with ABI-required alignment (e.g., x86_fp80, vectors on MSVC). - if (RD->hasAttr<AlignedAttr>()) { - AlignRequirement = AlignRequirementKind::RequiredByRecord; - } else { - // Check if any field requires alignment preservation. - AlignRequirement = AlignRequirementKind::None; - for (const auto *Field : RD->fields()) { - TypeInfo FI = getTypeInfo(Field->getType().getTypePtr()); - if (FI.AlignRequirement == AlignRequirementKind::RequiredByABI || - FI.AlignRequirement == AlignRequirementKind::RequiredByRecord || - typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) { - AlignRequirement = AlignRequirementKind::RequiredByABI; - break; - } - } - } + AlignRequirement = RD->hasAttr<AlignedAttr>() + ? AlignRequirementKind::RequiredByRecord + : AlignRequirementKind::None; break; } @@ -2696,21 +2663,6 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const { break; } - // Mark types that require ABI-correct alignment preservation under pragma - // pack. - if (AlignRequirement == AlignRequirementKind::None) { - const llvm::Triple &Triple = Target->getTriple(); - if (Triple.isOSWindows() && Triple.isWindowsMSVCEnvironment()) { - // x86_fp80 requires 16-byte alignment for correctness with movaps. - if (const auto *BT = T->getAs<BuiltinType>()) { - if (BT->getKind() == BuiltinType::LongDouble && - &Target->getLongDoubleFormat() == - &llvm::APFloat::x87DoubleExtended()) - AlignRequirement = AlignRequirementKind::RequiredByABI; - } - } - } - assert(llvm::isPowerOf2_32(Align) && "Alignment must be power of 2"); return TypeInfo(Width, Align, AlignRequirement); } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index dd8a019bd7b3b..659ebccd93a70 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -559,41 +559,6 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects( typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy; -/// Check if we should prevent MaxFieldAlignment from reducing this field's -/// alignment. Returns true if the field has ABI-required alignment -/// (e.g., x86_fp80, SIMD vectors on Windows), unless the struct has explicit -/// __attribute__((packed)). -static bool ShouldPreserveFieldAlignment(const ASTContext &Context, - const FieldDecl *FD, - AlignRequirementKind AlignReq, - bool StructHasPackedAttr) { - if (AlignReq == AlignRequirementKind::RequiredByABI || - AlignReq == AlignRequirementKind::RequiredByRecord) - return true; - - // Check if type requires alignment preservation under #pragma pack - // (but respect explicit __attribute__((packed))). - if (!StructHasPackedAttr && - Context.typeRequiresPreserveAlignUnderPragmaPack(FD->getType())) - return true; - return false; -} - -/// Check if a record contains any fields requiring alignment preservation. -/// Returns false if the record has explicit __attribute__((packed)). -static bool RecordContainsAlignPreservingFields(const ASTContext &Context, - const RecordDecl *RD) { - if (RD->hasAttr<PackedAttr>()) - return false; - for (const auto *Field : RD->fields()) { - TypeInfo TI = Context.getTypeInfo(Field->getType()); - if (TI.isAlignRequired() || - Context.typeRequiresPreserveAlignUnderPragmaPack(Field->getType())) - return true; - } - return false; -} - class ItaniumRecordLayoutBuilder { protected: // FIXME: Remove this and make the appropriate fields public. @@ -2064,19 +2029,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, UnpackedFieldAlign = std::max(UnpackedFieldAlign, MaxAlignmentInChars); // The maximum field alignment overrides the aligned attribute. - // However, do not reduce alignment for ABI-required alignments (e.g., - // x86_fp80, vector types) which must be preserved for correctness. - // On Windows MSVC, vectors where size equals alignment need their natural - // alignment preserved under #pragma pack. However, honor explicit - // __attribute__((packed)) on the struct (Packed=true means the struct - // has the packed attribute, not the field). - if (!MaxFieldAlignment.isZero() && - !ShouldPreserveFieldAlignment(Context, D, AlignRequirement, Packed)) { + if (!MaxFieldAlignment.isZero()) { PackedFieldAlign = std::min(PackedFieldAlign, MaxFieldAlignment); PreferredAlign = std::min(PreferredAlign, MaxFieldAlignment); UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment); } + if (!FieldPacked) FieldAlign = UnpackedFieldAlign; if (DefaultsToAIXPowerAlignment) @@ -2789,15 +2748,8 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } - // Respect pragma pack, attribute pack and declspec align. - // However, do not reduce alignment for ABI-required alignments (e.g., - // x86_fp80, vector types) which must be preserved for correctness. - bool StructHasPackedAttr = - FD->getParent() && FD->getParent()->hasAttr<PackedAttr>(); - if (!MaxFieldAlignment.isZero() && - !ShouldPreserveFieldAlignment(Context, FD, TInfo.AlignRequirement, - StructHasPackedAttr)) + if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); if (FD->hasAttr<PackedAttr>()) Info.Alignment = CharUnits::One(); @@ -3335,13 +3287,7 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { if (!RequiredAlignment.isZero()) { Alignment = std::max(Alignment, RequiredAlignment); auto RoundingAlignment = Alignment; - - // Check if this struct contains fields requiring alignment preservation - // (x86_fp80, vectors) before allowing MaxFieldAlignment (from #pragma pack) - // to reduce the overall struct alignment. However, if the struct has - // __attribute__((packed)), honor that explicit request. - if (!MaxFieldAlignment.isZero() && - !RecordContainsAlignPreservingFields(Context, RD)) + if (!MaxFieldAlignment.isZero()) RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment); RoundingAlignment = std::max(RoundingAlignment, RequiredAlignment); Size = Size.alignTo(RoundingAlignment); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index c5dcdee7dc5dd..f47fa7456b182 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19668,9 +19668,43 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, PPC().CheckPPCMMAType(T, NewFD->getLocation())) NewFD->setInvalidDecl(); - if (Context.getTargetInfo().hasAMDGPUTypes()) { - if (!AMDGPU().checkAMDGPUTypeSupport(T, NewFD->getLocation())) - NewFD->setInvalidDecl(); + // Under the Microsoft ABI, fp80 and vector typed fields have native + // alignment, even in packed structs. This behavior can be overridden by an + // explicit packed attribute on the field decl. + if (!InvalidDecl && !NewFD->hasAttr<PackedAttr>() && + Context.getTargetInfo().getTriple().isOSWindows() && + Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) { + auto AddImplicitAlignedAttr = [&]() { + TypeInfo TI = Context.getTypeInfo(T); + IntegerLiteral *AlignExpr = IntegerLiteral::Create( + Context, llvm::APInt(32, TI.Align / Context.getCharWidth()), + Context.getIntTypeForBitwidth(32, /*Signed=*/0), SourceLocation()); + NewFD->addAttr(AlignedAttr::CreateImplicit( + Context, /*IsAlignmentExpr=*/true, AlignExpr, SourceRange(), + AlignedAttr::GNU_aligned)); + }; + + // Check for x86_fp80 (long double with x87 format) + if (const auto *BT = T->getAs<BuiltinType>()) { + if (BT->getKind() == BuiltinType::LongDouble && + &Context.getTargetInfo().getLongDoubleFormat() == + &llvm::APFloat::x87DoubleExtended()) { + AddImplicitAlignedAttr(); + } + } else if (const auto *VT = T->getAs<VectorType>()) { + // Check for standard SIMD vectors where size == alignment (e.g., __m128, + // __m256, __m512). Only apply to power-of-2 sized vectors >= 128 bits + // (SSE minimum); smaller vectors and non-standard sizes can use reduced + // alignment. + if (VT->getVectorKind() == VectorKind::Generic) { + TypeInfo TI = Context.getTypeInfo(T); + // Check if it's a standard SIMD size: power of 2, >= 128 bits + if (TI.Width == TI.Align && TI.Width >= 128 && + llvm::isPowerOf2_64(TI.Width)) { + AddImplicitAlignedAttr(); + } + } + } } NewFD->setAccess(AS); >From 94d5102bf7cc247c636e76ec9c727408e8736e0d Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 10 Aug 2026 06:02:41 -0700 Subject: [PATCH 14/17] Addressed review comments --- clang/lib/AST/RecordLayoutBuilder.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 659ebccd93a70..f4541ff4e2c21 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2748,7 +2748,6 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( std::max(RequiredAlignment, std::max(DirectFieldAlignment, FieldTypeRequiredAlignment)); } - // Respect pragma pack, attribute pack and declspec align. if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); if (FD->hasAttr<PackedAttr>()) >From 04b79429da19a79713f5ff14c8344a6c029940d3 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 10 Aug 2026 08:18:36 -0700 Subject: [PATCH 15/17] Fixed test fail --- clang/lib/Sema/SemaDecl.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f47fa7456b182..799c92526b239 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19668,6 +19668,11 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, PPC().CheckPPCMMAType(T, NewFD->getLocation())) NewFD->setInvalidDecl(); + if (Context.getTargetInfo().hasAMDGPUTypes()) { + if (!AMDGPU().checkAMDGPUTypeSupport(T, NewFD->getLocation())) + NewFD->setInvalidDecl(); + } + // Under the Microsoft ABI, fp80 and vector typed fields have native // alignment, even in packed structs. This behavior can be overridden by an // explicit packed attribute on the field decl. >From 62a6dd0caf4bdf99166bb52b3232ac2277057450 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 10 Aug 2026 12:30:36 -0700 Subject: [PATCH 16/17] Fix failing test --- clang/lib/Sema/SemaDecl.cpp | 5 +++-- clang/test/CodeGen/ext-vector-member-alignment.c | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 799c92526b239..c6949147cc7d0 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19674,9 +19674,10 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, } // Under the Microsoft ABI, fp80 and vector typed fields have native - // alignment, even in packed structs. This behavior can be overridden by an - // explicit packed attribute on the field decl. + // alignment, even under #pragma pack. This behavior can be overridden by an + // explicit packed attribute on the field or struct. if (!InvalidDecl && !NewFD->hasAttr<PackedAttr>() && + Record && !Record->hasAttr<PackedAttr>() && Context.getTargetInfo().getTriple().isOSWindows() && Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) { auto AddImplicitAlignedAttr = [&]() { diff --git a/clang/test/CodeGen/ext-vector-member-alignment.c b/clang/test/CodeGen/ext-vector-member-alignment.c index 1eb6de32109cb..16598b700a449 100644 --- a/clang/test/CodeGen/ext-vector-member-alignment.c +++ b/clang/test/CodeGen/ext-vector-member-alignment.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -o - %s | FileCheck %s typedef float float4 __attribute__((ext_vector_type(4))); >From 9e519696309574002299028fe41218cf058fe915 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Mon, 10 Aug 2026 14:09:54 -0700 Subject: [PATCH 17/17] Fix format --- clang/lib/Sema/SemaDecl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index c6949147cc7d0..fb3e6dce4b3fc 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19676,8 +19676,8 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, // Under the Microsoft ABI, fp80 and vector typed fields have native // alignment, even under #pragma pack. This behavior can be overridden by an // explicit packed attribute on the field or struct. - if (!InvalidDecl && !NewFD->hasAttr<PackedAttr>() && - Record && !Record->hasAttr<PackedAttr>() && + if (!InvalidDecl && !NewFD->hasAttr<PackedAttr>() && Record && + !Record->hasAttr<PackedAttr>() && Context.getTargetInfo().getTriple().isOSWindows() && Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) { auto AddImplicitAlignedAttr = [&]() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
