RFR: 8333840: C2 SuperWord: wrong result for MulAddS2I when inputs permuted [v3]

Vladimir Kozlov kvn at openjdk.org
Wed Jun 12 15:41:15 UTC 2024


On Wed, 12 Jun 2024 13:08:38 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> This is an older bug, I discovered it by manual code inspection while refactoring.
>> 
>> I added some regression tests that produced wrong results.
>> 
>> You should understand the fix given the comments in the code, if not: tell me how to improve them ;)
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   more refactoring

Few comments

src/hotspot/share/opto/superword.cpp line 2815:

> 2813:   if (VectorNode::is_muladds2i(use)) {
> 2814:     assert(1 <= u_idx && u_idx <= 4, "expected range for the 4 inputs");
> 2815:     return _packset.is_muladds2i_pack_with_pack_inputs(u_pk);

Do we really need this assert? `is_muladds2i()` checks that use is MulAddS2I which by definition has 4 data inputs + control.

src/hotspot/share/opto/superword.cpp line 2837:

> 2835: // Inputs:                 1    2    3    4
> 2836: // Offsets:                0    0    1    1
> 2837: //   v = MulAddS2I(a, b) = a0 * b0 + a1 + b1

Typo, should be `a1 * b1`

src/hotspot/share/opto/superword.cpp line 2846:

> 2844: //   v = MulAddS2I(a, b) = a0 * b0 + b1 + a1     (case 2)
> 2845: //   v = MulAddS2I(a, b) = a1 * b1 + a0 + b0     (case 3)
> 2846: //   v = MulAddS2I(a, b) = a1 * b1 + b0 + a0     (case 4)

The same typo for second pair.

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

PR Review: https://git.openjdk.org/jdk/pull/19619#pullrequestreview-2113389085
PR Review Comment: https://git.openjdk.org/jdk/pull/19619#discussion_r1636699429
PR Review Comment: https://git.openjdk.org/jdk/pull/19619#discussion_r1636684346
PR Review Comment: https://git.openjdk.org/jdk/pull/19619#discussion_r1636684449


More information about the hotspot-compiler-dev mailing list