clang-cl miscompiles TSpatialHashGridPoints when using /Zp8 - why is that option used?

Development Pipeline & Plugins
, ,
1

Hello, we are compiling our game on Windows with Clang. After a particular cloth asset was submitted, the game started to crash when an actor with that asset was loaded. We tracked it down to an issue with Clang (see https://github.com/llvm/llvm\-project/issues/134694 ) - in these specific circumstances (clang-cl, /Zp8, actual SIMD type wrapped in a template), it ignores the alignment requirement (16 bytes) of TSpatialHashGridPoints::CellSize, aligning it on 8 bytes instead, and that causes a crash when doing the division:

divps xmm0,xmmword ptr [r8+28h]

This can be worked around by adding a redundant alignas specifier to the CellSize, as detailed in the LLVM Project GitHub issue, however what interests me is that given that the problem only manifests when using clang-cl and passing it the /Zp8 flag, why is it passed in the first place? It is being added in VCToolChain.cs since before 5.0. MSVC documentation ( https://learn.microsoft.com/en\-us/cpp/build/reference/zp\-struct\-member\-alignment?view\=msvc\-170 ) specifically says _not_ to use non-default alignment when including Windows headers - was the opposite a requirement at some point?

2

Steps to Reproduce

  1. Author a cloth asset using “self collision sphere constraints”.
  2. Use it in a level.
  3. Build the game for Windows with Clang, using at least the Development config.
  4. Run the game, loading the level containing the asset.
3

I looked at the history of VCToolChain.cs and /Zp8 has been passed for Win64 compiles since March 2012. We’ll need to do more investigation as to if it can be safely removed, there may be wide reaching implications changing something that has been set for well over a decade.

4

https://github.com/EpicGames/UnrealEngine/commit/f3a0b9afa9ca1effbc5bd9e97c0ee642f817cef1 I put this on a TargetRules setting for now. We won’t be able to address this any further for 5.6 but we’ll investigate more in the future.