[NativeAOT] Respect Align8 flag for thread statics #105905
Conversation
Co-authored-by: yowl <scott.waye@hubse.com>
| MethodTable *pMethodTable = (MethodTable*)gcDesc; | ||
| #if FEATURE_64BIT_ALIGNMENT | ||
| if (pMethodTable->RequiresAlign8) | ||
| { | ||
| return InternalCalls.RhpNewFastAlign8(pMethodTable); | ||
| } | ||
| #endif | ||
|
|
||
| return RuntimeImports.RhNewObject(pMethodTable); |
There was a problem hiding this comment.
Why does this work? AFAICT, RhNewObject already takes RequiresAlign8 into account.
When I looked at the root cause of this a while back, the problem was that RequiresAlign8 isn't set on the special gcDesc method tables (by the compiler, at least - I didn't check dynamic statics).
There was a problem hiding this comment.
Good point.
I will double check when I get to office with my ARM device. I assumed that if the test passes it should be all right but I could have overlooked something in the process.
There was a problem hiding this comment.
The relevant place to fix would be this one:
There was a problem hiding this comment.
@SingleAccretion Good catch!
the problem was that RequiresAlign8 isn't set on the special gcDesc method tables
dotnet/runtimelab#2609 worked around this problem by allocating all thread statics with 8-byte alignment.
There was a problem hiding this comment.
I agree we should revert it... I wanted to do a clean build on my RPi to figure out if I had some stale build artifacts... but unfortunately that turned out to be more painful than I expected. The combination of build tool updates and zlib-ng seems to hit a compiler bug now and I need to figure out first how to get past it:
[ 30%] Building C object _deps/fetchzlibng-build/CMakeFiles/zlib.dir/arch/arm/slide_hash_armv6.c.o
fatal error: error in backend: Cannot select: intrinsic %llvm.arm.uqsub16
There was a problem hiding this comment.
Turns out, it was indeed failing even on my machine. I suspect that I accidentally made the final change with adding the if (pMethodTable->RequiresAlign8) in a wrong VS Code window and I changed it on my local machine instead of the remote SSH session on the Raspberry Pi. I waited for the CI to be green but I also forgot that the inner loop doesn't run the smoke tests on ARM32.
I started working on a proper fix. Some early working attempt is here:
filipnavara@0ccb0d4. It's not exactly pretty. I'll sleep on it for a day or two before submitting it.
Contributes to #103234, #97729