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

Fei Gao fgao at openjdk.org
Mon Jul 11 01:37:39 UTC 2022


On Fri, 8 Jul 2022 20:32:24 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>>> 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.

Thanks for your review and test work, @vnkozlov .

May I have a second review please?

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

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


More information about the hotspot-compiler-dev mailing list