RFR: 8334431: C2 SuperWord: fix performance regression due to store-to-load-forwarding failures [v2]

[Dean Long]

On Mon, 18 Nov 2024 08:04:35 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> **History**
>> This issue became apparent with https://github.com/openjdk/jdk/pull/21521 / [JDK-8325155](https://bugs.openjdk.org/browse/JDK-8325155):
>> On machines that do not support sha intrinsics, we execute the sha code in java code. This java code has a loop that previously did not vectorize, but it now does since https://github.com/openjdk/jdk/pull/21521 / [JDK-8325155](https://bugs.openjdk.org/browse/JDK-8325155). It turns out that that kind of loop is actually slower when vectorized - this led to a regression, reported originally as:
>> `8334431: Regression 18-20% on Mac x64 on Crypto.signverify`
>> 
>> I then investigated the issue thoroughly, and discovered that it was even an issue before https://github.com/openjdk/jdk/pull/21521 / [JDK-8325155](https://bugs.openjdk.org/browse/JDK-8325155). I wrote a [blog-post ](https://eme64.github.io/blog/2024/06/24/Auto-Vectorization-and-Store-to-Load-Forwarding.html) about the issue.
>> 
>> **Summary of Problem**
>> 
>> As described in the [blog-post ](https://eme64.github.io/blog/2024/06/24/Auto-Vectorization-and-Store-to-Load-Forwarding.html), vectorization can introduce store-to-load failures that were not present in the scalar loop code. Where in scalar code, the loads and stores were all exactly overlapping or non-overlapping, in vectorized code they can now be partially overlapping. When a store and a later load are partially overlapping, the store value cannot be directly forwarded from the store-buffer to the load (would be fast), but has to first go to L1 cache. This incurs a higher latency on the dependency edge from the store to the load.
>> 
>> **Benchmark**
>> 
>> I introduced a new micro-benchmark in https://github.com/openjdk/jdk/pull/19880, and now further expanded it in this PR. You can see the extensive results in [this comment below](https://github.com/openjdk/jdk/pull/21521#issuecomment-2458938698).
>> 
>> The benchmarks look different on different machines, but they all have a pattern similar to this:
>> ![image](https://github.com/user-attachments/assets/3366f7fa-af44-44d4-a476-8cd0466fe937)
>> ![image](https://github.com/user-attachments/assets/1c1408c2-053e-4a8a-ad46-32b75b836161)
>> ![image](https://github.com/user-attachments/assets/d392c8cf-fb62-4593-93c7-a0d85ad5885e)
>> ![image](https://github.com/user-attachments/assets/3a79601f-4015-4f71-a510-cab7d7b59ed8)
>> 
>> We see that the `scalar` loop is faster for low `offset`, and the `vectorized` loop is faster for high offsets (and power-of-w offse...
>
> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 25 commits:
> 
>  - manual merge
>  - Merge branch 'master' into JDK-8334431-V-store-to-load-forwarding
>  - Merge branch 'master' into JDK-8334431-V-store-to-load-forwarding
>  - fix whitespace
>  - fix tests and build
>  - fix store-to-load forward IR rules
>  - updates before the weekend ... who knows if they are any good
>  - refactor to iteration threshold
>  - use jvmArgs again, and apply same fix as 8343345
>  - revert to jvmArgsPrepend
>  - ... and 15 more: https://git.openjdk.org/jdk/compare/543e355b...000f9f13

Why does the benchmark need to have so many methods, to make sure the different values are treated as constants?  I’m not sure, but JMH might turn @Param values into constants.  If so, then your benchmark can be greatly simplified.

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

PR Comment: https://git.openjdk.org/jdk/pull/21521#issuecomment-2484953605