clang-cl incorrectly interprets /Zp8 flag

[llvmbot]

Bugzilla Link	45420
Resolution	FIXED
Resolved on	Apr 16, 2020 10:31
Version	trunk
OS	Windows NT
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor,@zygoloid,@rnk

Extended Description

According to documentation,

https://docs.microsoft.com/en-us/cpp/build/reference/zp-struct-member-alignment?view=vs-2019

Visual studio interprets /Zp8 as a way to pack structure members, but it doesn't not change alignment of such struct itself.
clang-cl interprets it similar to -fpack-struct=8, and so /Zp8 flag is working significantly differently in msvc and clang-cl.

According to documentation WindowsSDK headers has to be compiled with /Zp8 flag, and so it can cause (and is causing in some projects) issues with using clang-cl as replacement for msvc.

the following sample:

#include <emmintrin.h>
struct A{__m128 a[1];};
static_assert(alignof(A) == 16, "should be aligned");

won't compile with /Zp8 flag with clang-cl, but it will in msvc.

Run-time sample:

https://godbolt.org/z/qh7Dbg

[llvmbot]

In a particular cases, that causes wrong code generation.

Cosider this sample:
struct A{
char a;
__m128 b;
};

alignment of b should be at least 16, even with /Zp8
And MSVC does it 16, while clang will folow -fpack-struct=8 rule, and make it 8.
Which is not only ABI incompatible, but also will lead to crash in runtime (aligned read from unaligned data).

See:

https://godbolt.org/z/ijqKzQ

moveaps will certainly crash, even if the code is used only in clang generated executable, without need of ABI compatibility.

In the same time WinSDK is compiled with /Zp8, which means alignment should be at least 8, not always 8.

[llvmbot]

related:

https://reviews.llvm.org/D57961
https://reviews.llvm.org/D4649

[rnk]

Clang seems to not propagate required alignment information to the parent class if the field is not of record type. Consider: https://gcc.godbolt.org/z/fdUfpb

struct __declspec(align(16)) AlignedStruct { int x; };
typedef int __declspec(align(16)) AlignedInt;
struct A { AlignedStruct a[1]; };
static_assert(alignof(A) == 16, "should be aligned");
struct B { AlignedInt a[1]; };
static_assert(alignof(B) == 16, "should be aligned");

MSVC accepts, clang only rejects the second assert. So we are getting it right when the aligned type is a struct. Hm.

[rnk]

Ah, the array type wrapper is the key. Arrays appear to not propagate the fact that their alignment was "required", meaning it came from an attribute. I have a patch to do that, and it seems to fix this.

[rnk]

https://reviews.llvm.org/D77754

There are some interesting related cases:

• _Atomic aligned types
• Vectors of aligned types
• ?

These are not that interesting, honestly. It's unreasonable to make a vector where the individual elements are over or underaligned. _Atomic variables have their own alignment. MSVC doesn't implement _Atomic, and it's not clear how their packing algorithm would interact with _Atomic if it did.

Something else I discovered is that clang supports underaligning types with __declspec(align()), but MSVC does not. We inherited this behavior from GCC. This could be considered a bug, but it's not the one you reported.

[llvmbot]

Thank you, looking forward for merge of the patch.

[rnk]

Pushed as 8da5b90