RFR: 8324655: Identify integer minimum and maximum patterns created with if statements [v2]

[Emanuel Peter]

On Fri, 26 Jan 2024 16:05:45 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> Hi all, I've created this patch which aims to convert common integer mininum and maximum patterns created using if statements into Min and Max nodes. These patterns are usually in the form of `a > b ? a : b` and similar, as well as patterns such as `if (a > b) b = a;`. While this transform doesn't generally improve code generation it's own, it simplifies control flow and creates new opportunities for vectorization.
>> 
>> I've created a benchmark for the PR, and I've attached some data from my (Zen 3) machine:
>> 
>>                                                 Baseline                     Patch            Improvement
>> Benchmark                   Mode   Cnt  Score      Error  Units    Score       Error  Units
>> IfMinMax.testReductionInt   avgt   15  500.307 ±  16.687  ns/op    509.383 ±  32.645  ns/op   (no change)*
>> IfMinMax.testReductionLong  avgt   15  493.184 ±  17.596  ns/op    513.587 ±  28.339  ns/op   (no change)*
>> IfMinMax.testSingleInt      avgt   15    3.588 ±   0.540  ns/op      2.965 ±   1.380  ns/op   (no change)
>> IfMinMax.testSingleLong     avgt   15    3.673 ±   0.128  ns/op      3.506 ±   0.590  ns/op   (no change)
>> IfMinMax.testVectorInt      avgt   15  340.425 ±  13.123  ns/op     59.689 ±   7.509  ns/op   + 5.7x
>> IfMinMax.testVectorLong     avgt   15  326.420 ±  15.554  ns/op    117.190 ±   5.622  ns/op   + 2.8x
>> 
>> 
>> * After writing this benchmark I discovered that the compiler doesn't seem to create some simple min/max reductions, even when using Math.min/max() directly. Is this known or should I create a followup RFE for this?
>> 
>> The patch passes tier 1-3 testing on linux x64. Reviews or comments would be appreciated!
>
> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Don't transform highly predictable branches

Yes, you would certainly run into this:
[JDK-8307516](https://bugs.openjdk.org/browse/JDK-8307516): C2 SuperWord: reconsider Reduction heuristic for UnorderedReduction

I'll get to it eventually, hopefully in the next few months.

Feel free to make this change (hack), and see if that would get you the desired results:

diff --git a/src/hotspot/share/opto/superword.cpp b/src/hotspot/share/opto/superword.cpp
index dd3f1d10dee..781458f4fd9 100644
--- a/src/hotspot/share/opto/superword.cpp
+++ b/src/hotspot/share/opto/superword.cpp
@@ -1987,7 +1987,7 @@ bool SuperWord::profitable(Node_List* p) {
   if (is_marked_reduction(p0)) {
     Node* second_in = p0->in(2);
     Node_List* second_pk = my_pack(second_in);
- if ((second_pk == nullptr) || (_num_work_vecs == _num_reductions)) {
+ if (second_pk == nullptr) {
       // Unmark reduction if no parent pack or if not enough work
       // to cover reduction expansion overhead
       _loop_reductions.remove(p0->_idx);

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

PR Comment: https://git.openjdk.org/jdk/pull/17574#issuecomment-1931612799