Skip to content
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

JIT: Small primitives on arm32/apple arm64 ABI need explicit normalization to 32 bits #101046

Closed
jakobbotsch opened this issue Apr 15, 2024 · 1 comment · Fixed by #106314
Closed
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@jakobbotsch
Copy link
Member

jakobbotsch commented Apr 15, 2024

ARM32 and Apple ARM64 ABIs both require normalization up to 32 bits for primitives. For example, a ushort must always be zero extended to 32 bits and short must always be sign extended.
The JIT does not explicitly insert this normalization anywhere. The following example using inline IL shows bad codegen on arm32 and apple arm64 because of it:

[MethodImpl(MethodImplOptions.NoInlining)]
public static void Foo(delegate*<short, int> test, ref ushort p)
{
    IL.Push(p);
    IL.Push(test);
    IL.Emit.Calli(new StandAloneMethodSig(System.Runtime.InteropServices.CallingConvention.Cdecl, typeof(int), typeof(short)));
    IL.Pop(out int value);
}

arm32 codegen:

G_M18803_IG02:  ;; offset=0x000E
00000E      add     r4, sp, 4
000010      movw    r3, 0x33f4
000014      movt    r3, 0x2f3
000018      blx     r3          // CORINFO_HELP_INIT_PINVOKE_FRAME
00001A      ldrh    r0, [r7]     ; needs additional sign extension
00001C      movs    r3, 0
00001E      str     r3, [sp+0x0C]
000020      movw    r3, LOW ADDRESS G_M18803_IG04
000024      movt    r3, HIGH ADDRESS G_M18803_IG04
000028      str     r3, [sp+0x14]
00002A      movs    r3, 0
00002C      strb    r3, [r5+0x04]
                                                ;; size=32 bbWeight=1 PerfScore 12.00
G_M18803_IG03:  ;; offset=0x002E
00002E      blx     r8

apple arm64 codegen:

G_M49066_IG02:  ;; offset=0x0024
            add     x0, fp, #32	// [V05 PInvokeFrame+0x08]
            mov     x1, xzr
            bl      CORINFO_HELP_INIT_PINVOKE_FRAME
            mov     x21, x0
            mov     x0, sp
            str     x0, [fp, #0x40]	// [V05 PInvokeFrame+0x28]
            mov     x0, fp
            str     x0, [fp, #0x50]	// [V05 PInvokeFrame+0x38]
            ldrh    w0, [x19]                       ; needs additional sign extension
            adr     x1, [G_M49066_IG04]
            str     x1, [fp, #0x48]	// [V05 PInvokeFrame+0x30]
            add     x1, fp, #32	// [V05 PInvokeFrame+0x08]
            str     x1, [x21, #0x08]
            strb    wzr, [x21, #0x04]
						;; size=56 bbWeight=1 PerfScore 12.50

G_M49066_IG03:  ;; offset=0x005C
            blr     x20

This issue is probably not reproducible in IL created from C# since the JIT generally keeps values zero/sign extended to 32 bits.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 15, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@jakobbotsch jakobbotsch added this to the 9.0.0 milestone Apr 15, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Apr 15, 2024
@jakobbotsch jakobbotsch self-assigned this Apr 15, 2024
@jakobbotsch jakobbotsch added the Priority:2 Work that is important, but not critical for the release label May 1, 2024
@jakobbotsch jakobbotsch removed the Priority:2 Work that is important, but not critical for the release label May 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 13, 2024
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Aug 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
1 participant