RFR: 8288883: C2: assert(allow_address || t != T_ADDRESS) failed after JDK-8283091

[Vladimir Kozlov]

On Fri, 8 Jul 2022 01:43:11 GMT, Fei Gao <fgao at openjdk.org> wrote:

>> In which call to `adjust_alignment_for_type_conversion()` you got AddP node?
>> Should we add checks there too?
>
>> In which call to `adjust_alignment_for_type_conversion()` you got AddP node? Should we add checks there too?
> 
> Thanks for your review, @vnkozlov .
> 
> When we called `adjust_alignment_for_type_conversion()` in `SuperWord::follow_def_uses()`, https://github.com/openjdk/jdk/blob/3f1174aa4709aabcfde8b40deec88b8ed466cc06/src/hotspot/share/opto/superword.cpp#L1525, we got AddP node. In this function, we also call `stmts_can_pack()` on the next line, which has checks to prevent unwanted pairs, https://github.com/openjdk/jdk/blob/3f1174aa4709aabcfde8b40deec88b8ed466cc06/src/hotspot/share/opto/superword.cpp#L1202. Maybe we don't have to add one more. WDYT?

@fg1417   `stmts_can_pack()` is called in an other place which is preceded by `are_adjacent_refs()` call which also has primitive type check (but different). I was thinking to convert checks in `stmts_can_pack()` to `assert`. But, on other hand, `is_java_primitive(bt)` is cheap and I would prefer to keep checks in `stmts_can_pack()` as they are in case we call it in an other place.

Anyway. After looking on code I agree with your current changes. Let me test it. And you need second review.

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

PR: https://git.openjdk.org/jdk/pull/9391