RFR: 8320725: C2: Add "is_associative" flag for floating-point add-reduction [v3]

Emanuel Peter epeter at openjdk.org
Mon Mar 18 12:59:33 UTC 2024


On Mon, 18 Mar 2024 12:38:55 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Bhavana Kilambi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Naming changes: replace strict/non-strict with more technical terms
>
> src/hotspot/share/opto/loopopts.cpp line 4306:
> 
>> 4304: }
>> 4305: 
>> 4306: static bool is_unordered_reduction(Node* n) {
> 
> I think you should rename all the "unordered" to "associative".
> I implemented this, and I think it would now be nicer to make that change.

You can do that also for the descriptions here, and the `move_unordered_reduction_out_of_loop` -> `move_associative_reduction_out_of_loop`

> src/hotspot/share/opto/vectornode.cpp line 1332:
> 
>> 1330:   case Op_AddReductionVL: return new AddReductionVLNode(ctrl, n1, n2);
>> 1331:   case Op_AddReductionVF: return new AddReductionVFNode(ctrl, n1, n2, is_associative);
>> 1332:   case Op_AddReductionVD: return new AddReductionVDNode(ctrl, n1, n2, is_associative);
> 
> Why do you only do it for the `F/D` `Add` instructions, but not the `Mul` instructions? Would those not equally profit from associativity?

I'm not super familiar with the Vector API, but I could not see that MUL is not associative.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18034#discussion_r1528462038
PR Review Comment: https://git.openjdk.org/jdk/pull/18034#discussion_r1528478789


More information about the hotspot-compiler-dev mailing list