-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NativeAOT] 32-bit platform ObjWriter and bit rot fixes #96890
Conversation
…dends to account for R2R ABI differences; use inline addend for 32-bit platforms; mark function symbols as Thumb code on ARM
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue Detailsnull
|
…rned; the should not get the symbol size or the Thumb bit
if (destination <= Register.R7) | ||
{ | ||
Builder.EmitShort(unchecked((short)(0x4478u + (byte)destination))); | ||
} | ||
else | ||
{ | ||
Builder.EmitShort(unchecked((short)(0x44f0u + (byte)destination))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review note: This affects R2R. It generates the same pattern as the JIT but it's one extra instruction. If you feel like we need to special-case it, I'll try to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine. This pattern is better for private working set.
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/Dwarf/DwarfCie.cs
Show resolved
Hide resolved
<CrossCompileArch Condition="$(CrossCompileRid.EndsWith('-arm'))">arm</CrossCompileArch> | ||
|
||
<CrossCompileAbi>gnu</CrossCompileAbi> | ||
<CrossCompileAbi Condition="$(CrossCompileRid.EndsWith('-arm'))">gnueabihf</CrossCompileAbi> | ||
|
||
<TargetTriple /> | ||
<TargetTriple Condition="'$(CrossCompileArch)' != ''">$(CrossCompileArch)-linux-gnu</TargetTriple> | ||
<TargetTriple Condition="'$(CrossCompileArch)' != ''">$(CrossCompileArch)-linux-$(CrossCompileAbi)</TargetTriple> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@am11 care to review this part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good and neat. Other related/interesting triplets are:
arm-linux-androideabi
arm-unknown-linux-gnueabi
arm-unknown-linux-gnueabihf
arm-unknown-linux-musleabi
arm-unknown-linux-musleabihf
armv7-unknown-linux-gnueabi
armv7-unknown-linux-gnueabihf
armv7-unknown-linux-musleabi
armv7-unknown-linux-musleabihf
of which, maybe <CrossCompileAbi Condition="'$(CrossCompileRid)' == 'linux-musl-arm'">musleabihf</CrossCompileAbi>
could be added? We have prereq docker image available for it as well.
Side note:
AFAIK, triplet is considered an archaic concept by some toolchain enthusiasts. They recommend using explicit -march
, -mtune
, -mabi
, -mcpu
and -mfpu
options. At some point we can delve into that and bring nativeaot and mono targets to new age. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'll have some follow up PRs later on, so I can add the "musl" then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, take your time. Basically the next line (49) can be deleted after that, since alpine
is unnecessary and more restrictive than unknown
or not setting the vendor at all; which we have for gnu and bionic.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@@ -203,19 +197,20 @@ void Compiler::unwindBegPrologCFI() | |||
|
|||
void Compiler::unwindPushPopMaskCFI(regMaskTP regMask, bool isFloat) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review note: This method is only ever called for TARGET_ARM
. The order of registers was opposite to the one expected in DWARF. Along with the change in unwindPushPopCFI
it now fixes the ARM target to generate correct DWARF descriptors.
Both of these issues were previously masked by the counter part code in ObjWriter that converted DWARF codes to EHABI. Notably it reversed the register order at each code offset, and the .regsave
opcode in the EHABI assembly automatically moved the call frame pointer by the register size.
Rough status of the linux-arm bring up:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
I have a fix/workaround that I will submit in subsequent PR. There's a bug in the LLD linker in the way how it resolves |
license/cla bot is stuck... |
* Use PC-relative relocations for ARMEmitter.EmitMOV(dest, symbol) * Fix handling of signed offsets in IMAGE_REL_BASED_THUMB_BRANCH24 calculations * Generate position independent code in NativeAOT on linux-arm * Handle ARM32 in DwarfCie * Fix relocations emitted in 32-bit ELF for x86/arm32; apply correct addends to account for R2R ABI differences; use inline addend for 32-bit platforms; mark function symbols as Thumb code on ARM * ELF/ARM32: Emit thumb mapping symbols for executable sections * Try to revert ARMEmitter.EmitMOV * Convert IMAGE_REL_BASED_THUMB_MOV32_PCREL to ELF with the same offset as R2R * Unoptimal, but working, version of INLINE_GET_TLS_VAR for ARM32 * Use PC-relative relocations for ARMEmitter.EmitMOV(dest, symbol) * Fat function pointers are not function symbols as far as ELF is concerned; the should not get the symbol size or the Thumb bit * Fix some bits and pieces of the ARM unwinding code * Don't try to use ObjWriter package on unsupported platforms * Generate valid ARM32 DWARF unwind info in JIT * Handle negative offsets in CFI_REL_OFFSET conversion (unused at the moment) * Add linux-arm support to cross-compile targets * Update src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc Co-authored-by: Jan Kotas <jkotas@microsoft.com> * Update ObjectWriter.cs Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com> * Fix the order of register push in CFI unwind codes on ARM. * Make jit-format happy without making the code look ugly --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
Fixes enough of the code to get "Hello World" up and running on NativeAOT / linux-arm. I was testing on the "DwarfDump" smoke test and even the LLDB debugging works quite well. Got all the way to
RhThrowEx
which reaches the unimplemented parts of unwinding and exception handling.Generally speaking, it seems to be possible to use the DWARF unwinding by enabling the support here.
It still fails on the first attempt to unwind a frame with some bogus values in the registers, but it does seem to look up the LSDA pointers at least.