Skip to content

Conversation

@DanShaders
Copy link
Contributor

Before #71148, providing only -triple=x86_64-windows-gnu to cc1 did not set -mms-bitfields (-fms-layout-compatibility=microsoft). Therefore, MS-compatible layout was only triggered in true MSVC targets. This is not the case now, so we should only check if we are compiling for Windows to test to determine if MS layout will be used.

The change of behavior is harmless as it only affects direct invocations of cc1.

Before llvm#71148, providing only `-triple=x86_64-windows-gnu` to cc1
did not set `-mms-bitfields` (`-fms-layout-compatibility=microsoft`).
Therefore, MS-compatible layout was only triggered in true MSVC targets.
This is not the case now, so we should only check if we are compiling
for Windows to test to determine if MS layout will be used.

The change of behavior is harmless as it only affects direct invocations
of cc1.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Dec 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 15, 2025

@llvm/pr-subscribers-clang

Author: Dan Klishch (DanShaders)

Changes

Before #71148, providing only -triple=x86_64-windows-gnu to cc1 did not set -mms-bitfields (-fms-layout-compatibility=microsoft). Therefore, MS-compatible layout was only triggered in true MSVC targets. This is not the case now, so we should only check if we are compiling for Windows to test to determine if MS layout will be used.

The change of behavior is harmless as it only affects direct invocations of cc1.


Full diff: https://github.com/llvm/llvm-project/pull/172337.diff

1 Files Affected:

  • (modified) clang/test/Sema/struct-packed-align.c (+3-2)
diff --git a/clang/test/Sema/struct-packed-align.c b/clang/test/Sema/struct-packed-align.c
index d6d0724da49f8..dec4398a2578d 100644
--- a/clang/test/Sema/struct-packed-align.c
+++ b/clang/test/Sema/struct-packed-align.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
 // RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-gnu -verify
 // RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-scei-ps4 -verify
 // RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-sie-ps5 -verify
 
@@ -131,7 +132,7 @@ struct nS {
   nt start_lba;
 };
 
-#if defined(_WIN32) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
+#if defined(_WIN32)
 // Alignment doesn't affect packing in MS mode.
 extern int n1[sizeof(struct nS) == 16 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 8 ? 1 : -1];
@@ -159,7 +160,7 @@ struct packed_chars {
   char f : 4, g : 8, h : 8, i : 8;
 };
 
-#if (defined(_WIN32) || defined(__SCE__)) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
+#if defined(_WIN32) || defined(__SCE__)
 // On Windows clang uses MSVC compatible layout in this case.
 //
 // Additionally, test for pre-r254596 clang behavior on the PS4/PS5 targets.

@mstorsjo mstorsjo requested a review from erichkeane December 15, 2025 17:21
Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

I can confirm, that before 1760eff, bin/clang -target x86_64-windows-gnu -c ../../clang/test/Sema/struct-packed-align.c (without -cc1) also did fail, triggering these static asserts. So the actual user-facing behaviour is retained, and only the test with a plain -cc1 has changed.

@DanShaders
Copy link
Contributor Author

DanShaders commented Dec 15, 2025

Someone will have to merge this as I don't have write access

@erichkeane erichkeane merged commit e53acac into llvm:main Dec 15, 2025
12 checks passed
@DanShaders DanShaders deleted the fix-mingw-build branch December 15, 2025 17:55
@mstorsjo
Copy link
Member

Thanks! Btw @DanShaders, for future commits - please turn off Keep my email addresses private setting in your account. See LLVM Developer Policy and LLVM Discourse for more information.

(We have CI that is meant to point this out already when PRs are opened, but it doesn't really work as we'd want in all cases.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants