[clang] Fix mis alignment fp80 (PR #208256)
Zahira Ammarguellat via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 10 07:26:33 PDT 2026
https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/208256
>From b925b2c3fa0f15b20898bd6456329eddfdb027ea Mon Sep 17 00:00:00 2001
From: Zahira Ammarguellat <zahira.ammarguellat at intel.com>
Date: Wed, 20 May 2026 05:48:13 -0700
Subject: [PATCH 1/3] [OpenMP] Prevent parser infinite loop on unimplemented
clauses
---
clang/lib/Basic/OpenMPKinds.cpp | 1 +
clang/lib/Parse/ParseOpenMP.cpp | 9 ++
clang/lib/Sema/SemaOpenMP.cpp | 4 +
.../OpenMP/unimplemented_clause_messages.cpp | 93 +++++++++++++++++++
4 files changed, 107 insertions(+)
create mode 100644 clang/test/OpenMP/unimplemented_clause_messages.cpp
diff --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index 287eb217ba458..675d86349c933 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -965,6 +965,7 @@ void clang::getOpenMPCaptureRegions(
case OMPD_simd:
case OMPD_single:
case OMPD_target_data:
+ case OMPD_taskgraph:
case OMPD_taskgroup:
case OMPD_stripe:
// These directives (when standalone) use OMPD_unknown as the region,
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 45a47ec797f01..ba3d3113700ff 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2388,6 +2388,8 @@ StmtResult Parser::ParseOpenMPExecutableDirective(
ImplicitClauseAllowed = false;
Actions.OpenMP().StartOpenMPClause(CKind);
HasImplicitClause = false;
+ SourceLocation ClauseLoc = Tok.getLocation();
+
OMPClause *Clause =
ParseOpenMPClause(DKind, CKind, !SeenClauses[unsigned(CKind)]);
SeenClauses[unsigned(CKind)] = true;
@@ -2398,6 +2400,13 @@ StmtResult Parser::ParseOpenMPExecutableDirective(
if (Tok.is(tok::comma))
ConsumeToken();
Actions.OpenMP().EndOpenMPClause();
+
+ // If ParseOpenMPClause returned without consuming any tokens, skip
+ // to end to avoid an infinite loop.
+ if (Tok.getLocation() == ClauseLoc) {
+ skipUntilPragmaOpenMPEnd(DKind);
+ break;
+ }
}
// End location of the directive.
EndLoc = Tok.getLocation();
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index d6f6bc919a31b..76b40a5039180 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -6778,6 +6778,10 @@ StmtResult SemaOpenMP::ActOnOpenMPExecutableDirective(
case OMPD_begin_declare_variant:
case OMPD_end_declare_variant:
llvm_unreachable("OpenMP Directive is not allowed");
+ case OMPD_taskgraph:
+ Diag(StartLoc, diag::err_omp_unexpected_directive)
+ << 1 << getOpenMPDirectiveName(OMPD_taskgraph);
+ return StmtError();
case OMPD_unknown:
default:
llvm_unreachable("Unknown OpenMP directive");
diff --git a/clang/test/OpenMP/unimplemented_clause_messages.cpp b/clang/test/OpenMP/unimplemented_clause_messages.cpp
new file mode 100644
index 0000000000000..172203ea5d040
--- /dev/null
+++ b/clang/test/OpenMP/unimplemented_clause_messages.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -verify=expected,omp60 -fopenmp -fopenmp-version=60 %s
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 %s
+// RUN: %clang_cc1 -verify=expected,omp60 -fopenmp-simd -fopenmp-version=60 %s
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp-simd -fopenmp-version=51 %s
+
+
+void test_induction_basic() {
+ int i;
+ // omp60-warning at +4{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+ // omp60-error at +3{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+ // omp51-warning at +2{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+ // omp51-error at +1{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+#pragma omp parallel for induction(i)
+ for (i = 0; i < 10; ++i)
+ ;
+}
+
+void test_apply() {
+ // omp60-warning at +4{{extra tokens at the end of '#pragma omp tile' are ignored}}
+ // omp60-error at +3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp51-error at +2{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp51-warning at +1{{extra tokens at the end of '#pragma omp tile' are ignored}}
+#pragma omp tile sizes(10) apply(intratile: unroll)
+ for (int i = 0; i < 10; ++i)
+ ;
+}
+
+void test_empty_apply() {
+ // omp60-warning at +4{{extra tokens at the end of '#pragma omp tile' are ignored}}
+ // omp60-error at +3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp51-error at +2{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp51-warning at +1{{extra tokens at the end of '#pragma omp tile' are ignored}}
+#pragma omp tile sizes(10) apply()
+ for (int i = 0; i < 10; ++i)
+ ;
+}
+
+void test_nested_apply()
+{
+ // omp60-error at +5{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp60-warning at +4{{extra tokens at the end of '#pragma omp tile' are ignored}}
+ //omp51-error at +3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}}
+ // omp51-warning at +2{{extra tokens at the end of '#pragma omp tile' are ignored}}
+#pragma omp tile sizes(10) \
+ apply(intratile: unroll partial(2) apply(reverse))
+ for (int i = 0; i < 100; ++i)
+ ;
+}
+
+void test_induction_with_following_clause() {
+ int i;
+ // omp60-warning at +4{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+ // omp60-error at +3{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+ // omp51-error at +2{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+ // omp51-warning at +1{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+#pragma omp parallel for induction(i) num_threads(4)
+ for (i = 0; i < 10; ++i)
+ ;
+}
+
+class Point {
+ float x, y, m;
+ char color;
+
+};
+
+void processPointsInLine() {
+ float separation;
+ // omp60-error at +4{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+ // omp60-warning at +3{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+ // omp51-error at +2{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}}
+ // omp51-warning at +1{{extra tokens at the end of '#pragma omp parallel for' are ignored}}
+#pragma omp parallel for induction(step(Separation))
+ for (int i = 0; i < 10; ++i) {
+ ;
+ }
+}
+
+// Make sure test doesn't crash.
+void test_tasgraph()
+{
+ // omp60-error at +2{{unexpected OpenMP directive '#pragma omp taskgraph'}}
+ // omp51-error at +1{{unexpected OpenMP directive '#pragma omp taskgraph'}}
+#pragma omp taskgraph
+ for (int i = 0; i < 10; ++i)
+ ;
+}
+
+void test_implemented_clause() {
+#pragma omp tile sizes(10)
+ for (int i = 0; i < 10; ++i)
+ ;
+}
>From 51b5dd59154f0213e4052d9a8cb7365c7afde0fd Mon Sep 17 00:00:00 2001
From: Ammarguellat <zahira.ammarguellat at intel.com>
Date: Wed, 8 Jul 2026 09:24:56 -0700
Subject: [PATCH 2/3] [Clang] Fix x86_fp80 misalignment with pragma pack on
Windows
---
clang/lib/AST/RecordLayoutBuilder.cpp | 27 ++++++++-
clang/test/CodeGen/x86_fp80-alignment-win.cpp | 58 +++++++++++++++++++
2 files changed, 82 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 854f88f20b2b6..aa9161f5dccea 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2693,6 +2693,25 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
auto TInfo =
Context.getTypeInfoInChars(FD->getType()->getUnqualifiedDesugaredType());
ElementInfo Info{TInfo.Width, TInfo.Align};
+ // Types like x86_fp80 (long double with /Qlong-double) require 16-byte
+ // (or greater) for correctness.
+ bool ContainsX86FP80 = false;
+ std::function<bool(QualType)> CheckForX86FP80 = [&](QualType Ty) -> bool {
+ const Type *BaseType = Ty->getBaseElementTypeUnsafe();
+ if (const auto *BT = dyn_cast<BuiltinType>(BaseType)) {
+ return BT->getKind() == BuiltinType::LongDouble &&
+ &Context.getTargetInfo().getLongDoubleFormat() ==
+ &llvm::APFloat::x87DoubleExtended();
+ }
+ if (const auto *RT = dyn_cast<RecordType>(BaseType)) {
+ for (const auto *Field : RT->getDecl()->fields()) {
+ if (CheckForX86FP80(Field->getType()))
+ return true;
+ }
+ }
+ return false;
+ };
+ ContainsX86FP80 = CheckForX86FP80(FD->getType());
// Respect align attributes on the field.
CharUnits FieldRequiredAlignment =
Context.toCharUnitsFromBits(FD->getMaxAlignment());
@@ -2717,10 +2736,12 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
// Capture required alignment as a side-effect.
RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment);
}
- // 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();
Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment);
return Info;
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 6f8a42b0e9bc5aaf0e33b185751d9e9acd9d06c2 Mon Sep 17 00:00:00 2001
From: Ammarguellat <zahira.ammarguellat at intel.com>
Date: Fri, 10 Jul 2026 07:26:10 -0700
Subject: [PATCH 3/3] Leveraged FieldRequiredAlignment
---
clang/lib/AST/RecordLayoutBuilder.cpp | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index aa9161f5dccea..f0d9caf3c2469 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2693,9 +2693,12 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
auto TInfo =
Context.getTypeInfoInChars(FD->getType()->getUnqualifiedDesugaredType());
ElementInfo Info{TInfo.Width, TInfo.Align};
+ // Respect align attributes on the field.
+ CharUnits FieldRequiredAlignment =
+ Context.toCharUnitsFromBits(FD->getMaxAlignment());
+
// Types like x86_fp80 (long double with /Qlong-double) require 16-byte
// (or greater) for correctness.
- bool ContainsX86FP80 = false;
std::function<bool(QualType)> CheckForX86FP80 = [&](QualType Ty) -> bool {
const Type *BaseType = Ty->getBaseElementTypeUnsafe();
if (const auto *BT = dyn_cast<BuiltinType>(BaseType)) {
@@ -2711,10 +2714,10 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
}
return false;
};
- ContainsX86FP80 = CheckForX86FP80(FD->getType());
- // Respect align attributes on the field.
- CharUnits FieldRequiredAlignment =
- Context.toCharUnitsFromBits(FD->getMaxAlignment());
+ if (CheckForX86FP80(FD->getType()))
+ FieldRequiredAlignment =
+ std::max(FieldRequiredAlignment, CharUnits::fromQuantity(16));
+
// Respect align attributes on the type.
if (Context.isAlignmentRequired(FD->getType()))
FieldRequiredAlignment = std::max(
@@ -2739,9 +2742,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();
Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment);
return Info;
More information about the cfe-commits
mailing list