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