[foreign-memaccess] RFR 8237082: Workaround C2 limitations when working with long loops

[Jorn Vernee]

Hi,

This is great! I can confirm the speedup on my machine, as well as 
hoisting in the generated code for int and byte access loops.

Some comments:
- Added MH fields in AddressVarHandleGenerator can be 'final' as well.
- The size checks on the operands in addOffsets/multiplyOffsets or 
checking op1 against Integer.MIN_VALUE twice, instead of op1 once, and 
op2 once.
- In MemorySegmentImpl, I think at least the exception creation code 
could be factored into a helper method without affecting performance.

I think this is a nice proof of concept that shows where the problem 
lies, though like you say, the ultimate solution is to improve C2 (there 
are some missed cases where offset computations happen as well, e.g. in 
MA::addOffset, or MA::getUnsafeOffset. Those can be handled by JDK-8223051).

Cheers,
Jorn

On 14/01/2020 16:06, Maurizio Cimadamore wrote:
> Hi,
> both C2 and Graal do not like for loops with long computations inside 
> - in C2, the bound-check-elimination code (BCE) is extremely sensitive 
> to which opcodes are used when putting together offsets in a linear 
> expression [1]. So, whenever C2 sees a long opcode being used (e.g. 
> LMUL, or LADD) the BCE logic bails out, which means that in such cases 
> we get no bounds check hoisting.
>
> The medium/long term solution is, obviously, to fix C2 [2], so that it 
> works on long loops as well as with int loops - after all, this is 
> where many of the new APIs we are designing are headed.
>
> As a short term boost, I've put together a patch which classifies 
> segments as either small or large; if a segment is small, then we can 
> use some trickery to force int opcodes to be used in offset 
> computations instead of their long counterparts. Doing so removes most 
> of the performance bottlenecks associated with indexed var handle 
> access, some of which were visible in the synthetic benchmarks (see 
> LoopOverNonConstant [3]).
>
> Webrev:
>
> http://cr.openjdk.java.net/~mcimadamore/panama/8237082_v2/
>
> Cheers
> Maurizio
>
> [1] - 
> http://hg.openjdk.java.net/panama/dev/file/b94889c7e153/src/hotspot/share/opto/loopnode.cpp
> [2] - https://bugs.openjdk.java.net/browse/JDK-8223051
> [3] - 
> http://hg.openjdk.java.net/panama/dev/file/fb3c9c52fdff/test/micro/org/openjdk/bench/jdk/incubator/foreign/LoopOverNonConstant.java
>
>