JIT: Add an emitter peephole for post-indexed addressing #105181

jakobbotsch · 2024-07-20T14:23:36Z

This transforms sequences like

ldr x0, [x1]
add x1, x1, #8

into the equivalent

ldr x0, [x1], #8

The second half of this change will be having lowering and strength reduction set up the IR such that this transformation kicks in.

Example codegen:

public static ref int ArrRef(int[] arr)
{
    return ref MemoryMarshal.GetArrayDataReference(arr);
}

@@ -6,15 +6,14 @@ G_M1984_IG01:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref,
 
 G_M1984_IG02:        ; bbWeight=1, gcrefRegs=0001 {x0}, byrefRegs=0000 {}, byref
                              ; gcrRegs +[x0]
-            ldrsb   wzr, [x0]
-            add     x0, x0, #16
+            ldrsb   wzr, [x0], #0x10
                              ; gcrRegs -[x0]
                              ; byrRegs +[x0]
-						;; size=8 bbWeight=1 PerfScore 3.50
+						;; size=4 bbWeight=1 PerfScore 3.00
 
 G_M1984_IG03:        ; bbWeight=1, epilog, nogc, extend
             ldp     fp, lr, [sp], #0x10
             ret     lr
 						;; size=8 bbWeight=1 PerfScore 2.00
-; Total bytes of code: 24
+; Total bytes of code: 20

No pre-indexing support yet.

This transforms sequences like ```asm ldr x0, [x1] add x1, x1, dotnet#8 ``` into the equivalent ```asm ldr x0, [x1], dotnet#8 ``` The second half of this change will be having lowering and strength reduction set up the IR such that this transformation kicks in.

dotnet-policy-service · 2024-07-20T14:24:10Z

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

jakobbotsch · 2024-07-20T14:53:39Z

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-coreclr gcstress0x3-gcstress0xc

azure-pipelines · 2024-07-20T14:53:53Z

Azure Pipelines successfully started running 3 pipeline(s).

jakobbotsch · 2024-07-20T19:47:40Z

cc @dotnet/jit-contrib - sorry for the weekend ping, but would like to get this into preview 7.

jitstress failures are #105186, #105187

libraries-jitstress failures are #105092, #105189, #102706

gcstress failures are #105186, #105187

Diffs

tannergooding · 2024-07-20T20:11:51Z

src/coreclr/jit/emitarm64.cpp

@@ -5844,6 +5844,12 @@ void emitter::emitIns_R_R_I(instruction     ins,
                return;
            }

+            if ((reg1 == reg2) && (EA_SIZE(attr) == EA_PTRSIZE) && emitComp->opts.OptimizationEnabled() &&


This doesn't necessarily have to be EA_PTRSIZE, right? Rather, we just need the offset to be a multiple of 8 in range (looks to be in the range of -4096 to 4032, inclusive)?

-- Not something I think that needs to be handled in this PR, but rather that might be a possible future improvement on top.

Looks like it depends on the instruction. Some are multiples of 4/8/16 and some might be raw offsets. The ranges vary based on instruction too

It has to be EA_PTRSIZE -- the register that is written is the address that was used for the load, so it is always pointer sized.

The offset is an unscaled 9-bit signed immediate for the (single register) loads that support the write-back. So -256 to 255 is supported for the combined form.

It has to be EA_PTRSIZE -- the register that is written is the address that was used for the load, so it is always pointer sized.

👍

The offset is an unscaled 9-bit signed immediate for the (single register) loads that support the write-back. So -256 to 255 is supported for the combined form.

Yeah, I was looking at the wrong instruction here. There's a few different post-indexing forms. For ldp, as an example, it's:

For the 32-bit post-index and 32-bit pre-index variant: is the signed immediate byte offset, a multiple of 4 in the range -256 to 252, encoded in the "imm7" field as /4.
For the 64-bit post-index and 64-bit pre-index variant: is the signed immediate byte offset, a multiple of 8 in the range -512 to 504, encoded in the "imm7" field as /8.

There's then ldapr which is fixed 4 or 8, ldiapp which is fixed 8 or 16, and ldpsw which has the ... a multiple of 4 in the range -256 to 252 ... text

Seems we're not handling these ones in this PR, which is fine, just had a mental disconnect due to the differences between them 😄

Yeah, some of those forms we could definitely handle in the future. One complication is that some of those forms allow redefining the GC ness of up to 3 registers which instrDesc does not support today, so we would need to expand it in some way.

tannergooding · 2024-07-20T20:15:21Z

src/coreclr/jit/emitarm64.cpp

+        return false;
+    }
+
+    if ((emitLastIns->idInsFmt() != IF_LS_2A) || emitLastIns->idIsTlsGD())


Are there any load/store instructions this doesn't cover for the initial work being done?

There's a lot of different instructions that support post-indexing, but not sure if all of them are IF_LS_2A or not

Yeah, this doesn't support ldp and stp. Those instructions can have 7-bit scaled offsets.

There's a lot of different instructions that support post-indexing, but not sure if all of them are IF_LS_2A or not

I don't think there are any other instructions that load or stores that support the post-indexed writeback form, but I could be wrong. ldp and stp have support for post-indexing with writeback that we aren't supporting here yet, so that's something we could look into adding in the future.

src/coreclr/jit/emitarm64.cpp

tannergooding

Changes LGTM.

Would be nice to log an issue for LDP to be covered as well, but I don't think its important to handle in this PR

jakobbotsch · 2024-07-21T08:05:47Z

Would be nice to log an issue for LDP to be covered as well, but I don't think its important to handle in this PR

Good idea, I opened #105192 for that.

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 20, 2024

dotnet-policy-service bot assigned jakobbotsch Jul 20, 2024

Nit

75276fc

This was referenced Jul 20, 2024

'System.Net.NameResolution.Functional.Tests' Failure #105092

Closed

linux-armel checked CoreCLR_NonPortable failing to build with "Unable to find toolchain executable. Name: 'ar', Prefix: 'llvm-'" #105176

Closed

jakobbotsch mentioned this pull request Jul 20, 2024

JIT: Have lowering set up IR for post-indexed addressing and make strength reduced IV updates amenable to post-indexed addressing #105185

Merged

jakobbotsch marked this pull request as ready for review July 20, 2024 19:47

tannergooding reviewed Jul 20, 2024

View reviewed changes

src/coreclr/jit/emitarm64.cpp Outdated Show resolved Hide resolved

jakobbotsch added 2 commits July 20, 2024 22:31

Fix off by one

3efbdb1

Remove wrong comment

e8eba17

tannergooding approved these changes Jul 20, 2024

View reviewed changes

This was referenced Jul 21, 2024

TimeProviderTests.TestProviderTimer failed in CI #103459

Closed

System.Numerics.Tensors.Tests.TensorSpanTests test failure #103525

Closed

jakobbotsch changed the title ~~JIT: Add an emitter peephole for for post-indexed addressing~~ JIT: Add an emitter peephole for post-indexed addressing Jul 21, 2024

jakobbotsch merged commit fcb9b18 into dotnet:main Jul 21, 2024
101 of 108 checks passed

jakobbotsch deleted the post-indexed-addressing branch July 21, 2024 07:56

jakobbotsch mentioned this pull request Jul 21, 2024

JIT: Missing support for post-indexed addressing modes in stp and ldp #105192

Open

jakobbotsch mentioned this pull request Jul 21, 2024

JIT: Missing support for pre-indexed addressing on arm64 #105193

Open

github-actions bot locked and limited conversation to collaborators Aug 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: Add an emitter peephole for post-indexed addressing #105181

JIT: Add an emitter peephole for post-indexed addressing #105181

jakobbotsch commented Jul 20, 2024 •

edited

Loading

dotnet-policy-service bot commented Jul 20, 2024

jakobbotsch commented Jul 20, 2024

azure-pipelines bot commented Jul 20, 2024

jakobbotsch commented Jul 20, 2024 •

edited

Loading

tannergooding Jul 20, 2024

tannergooding Jul 20, 2024

jakobbotsch Jul 20, 2024

tannergooding Jul 20, 2024

jakobbotsch Jul 20, 2024

tannergooding Jul 20, 2024

jakobbotsch Jul 20, 2024

tannergooding left a comment

jakobbotsch commented Jul 21, 2024

JIT: Add an emitter peephole for post-indexed addressing #105181

JIT: Add an emitter peephole for post-indexed addressing #105181

Conversation

jakobbotsch commented Jul 20, 2024 • edited Loading

dotnet-policy-service bot commented Jul 20, 2024

jakobbotsch commented Jul 20, 2024

azure-pipelines bot commented Jul 20, 2024

jakobbotsch commented Jul 20, 2024 • edited Loading

tannergooding Jul 20, 2024

Choose a reason for hiding this comment

tannergooding Jul 20, 2024

Choose a reason for hiding this comment

jakobbotsch Jul 20, 2024

Choose a reason for hiding this comment

tannergooding Jul 20, 2024

Choose a reason for hiding this comment

jakobbotsch Jul 20, 2024

Choose a reason for hiding this comment

tannergooding Jul 20, 2024

Choose a reason for hiding this comment

jakobbotsch Jul 20, 2024

Choose a reason for hiding this comment

tannergooding left a comment

Choose a reason for hiding this comment

jakobbotsch commented Jul 21, 2024

jakobbotsch commented Jul 20, 2024 •

edited

Loading

jakobbotsch commented Jul 20, 2024 •

edited

Loading