RFR: 8313402: C1: Incorrect LoadIndexed value numbering [v2]

[Paul Hohensee]

On Tue, 1 Aug 2023 16:10:12 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> See the bug for more investigation. 
>> 
>> This manifests in current tests, if you run them with C1. The root cause for the failure is that we fold two `LoadIndexed` nodes, when one of them reads `char` from `byte[]` via `_getCharStringU` intrinsic, and another one reads `byte` normally. So we can fold the "char"-reading load with "byte" reading load, effectively reading the wrong thing. New regression test shows it: it would read "42" instead of full char.
>> 
>> 
>> $ build/macosx-aarch64-server-fastdebug/images/jdk/bin/java -XX:TieredStopAtLevel=1 -XX:CICompilerCount=1 -XX:+PrintCompilation -XX:+PrintIR0 -XX:+PrintValueNumbering Test8313402.java
>> 
>> . 8 0 i152 a141[i110](i144) (B) [rc]
>> ...
>> . 7 0 i180 a162[i110] (C)
>> ...
>> Value Numbering: LoadIndexed i180 equal to i152 (size 47, entries 27, nesting-diff 0)
>> ``` 
>> 
>> GVN hash discriminates on `type()->tag()`, but that `ValueType` maps to the same `T_INT` for both `char` and `byte`! Instead of hashing on that, let's hash on the original element type instead.
>> 
>> Testing:
>>  - [x] New regression test fails without the fix, passes after the fix
>>  - [x] Linux AArch64 fastdebug, `tier1 tier2 tier3`
>>  - [x] Linux x86_64 fastdebug, `tier1 tier2 tier3`
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8313402-c1-gvn-loadindexed
>  - Initial fix

Looks like the right thing to do.

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

Marked as reviewed by phh (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15091#pullrequestreview-1557373569