RFR: 8020282: Generated code quality: redundant LEAs in the chained dereferences

[Manuel Hässig]

On Wed, 4 Jun 2025 08:39:57 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> ## Summary
>> 
>> On x86, chained dereferences of narrow oops at a constant offset from the base oop can use a `lea` instruction to perform the address computation in one go using the `leaP8Narrow`, `leaP32Narrow`, and `leaPCompressedOopOffset` matching rules. However, the generated code contains an additional `lea` with an unused result:
>> 
>> ; OptoAssembly
>> 03d     decode_heap_oop_not_null R8,R10
>> 041     leaq    R10, [R12 + R10 << 3 + #12] (compressed oop addressing) ; ptr compressedoopoff32
>> 
>> ; x86
>> 0x00007f1f210625bd:   lea    (%r12,%r10,8),%r8        ; result is unused
>> 0x00007f1f210625c1:   lea    0xc(%r12,%r10,8),%r10    ; the same computation as decode, but with offset
>> 
>> 
>> This PR adds a peephole optimization to remove such redundant `lea`s.
>> 
>> ## The Issue in Detail
>> 
>> The ideal subgraph producing redundant `lea`s, or rather redundant `decodeHeapOop_not_null`s, is `LoadN -> DecodeN -> AddP`, where both the address and base edge of the `AddP` originate from the `DecodeN`. After matching, this becomes
>> 
>> LoadN -> decodeHeapOop_not_null -> leaP*
>>     ______________________________Î
>> 
>> where `leaP*` is either of `leaP8Narrow`, `leaP32Narrow`, or `leaPCompressedOopOffset` (depending on the heap location and size). Here, the base input of `leaP*` comes from the decode. Looking at the matching code path, we find that the `leaP*` rules match both the `AddP` and the `DecodeN`, since x86 can fold this, but the following code adds the decode back as the base input to `leaP*`:
>> 
>> https://github.com/openjdk/jdk/blob/c29537740efb04e061732a700582d43b1956cff4/src/hotspot/share/opto/matcher.cpp#L1894-L1897
>> 
>> On its face, this is completely unnecessary if we matched a `leaP*`, since it already computes the result of the decode,  so adding the `LoadN` node as base seems like the logical choice. However, if the derived oop computed by the `leaP*` gets added to an oop map, this `DecodeN` is needed as the base for the derived oop. Because as of now, derived oops in oop maps cannot have narrow base pointers.
>> 
>> This leaves us with a handful of possible solutions:
>>  1. implement narrow bases for derived oops in oop maps,
>>  2. perform some dead code elimination after we know which oops are part of oop maps,
>>  3. add a peephole optimization to simply remove unused `lea`s.
>> 
>> Option 1 would have been ideal in the sense, that it is the earliest possible point to remove the decode, which would simplify the graph and reduce pressure on the regi...
>
> test/micro/org/openjdk/bench/vm/compiler/x86/RedundantLeaPeephole.java line 33:
> 
>> 31: @Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
>> 32: @Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
>> 33: @Fork(value = 3, jvmArgsAppend = {"-Xms1g", "-Xmx1g"})
> 
> Ha, what did you need these args for? Could be nice to have a little comment in the code.

This is what I gather to be good practice from @shipilev's [blog post about JMS benchmarks](https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_benchmark). It ensures a consistent heap size across machines and runs because the `StoreN` benchmarks are sensitive to different GC's and heap layouts. But these are my first JMH benchmarks, so I appreciate any input.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25471#discussion_r2126107900