RFR: 8343206: Final graph reshaping should not compress abstract or interface class pointers [v3]

[Coleen Phillimore]

On Wed, 30 Oct 2024 11:53:40 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> @fisk found this problematic optimization in final graph reshaping where we would convert a `CmpP` into a `CmpN` by converting a constant class pointer operand to a narrow class pointer. After  [JDK-8338526](https://bugs.openjdk.org/browse/JDK-8338526), this is not valid if the class pointer refers to an interface or abstract class.
>> 
>> I think it's not an issue in current code though. The only way we can get a dynamic narrow class is when loading from an object at `oopDesc::klass_offset_in_bytes()`:
>> https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/type.cpp#L3521-L3522
>> 
>> Comparisons of such loads with a constant class pointer of interface or abstract class type are always folded during GVN:
>> https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/subnode.cpp#L1164-L1171
>> 
>> And therefore, the code in `Compile::final_graph_reshaping_main_switch` will never trigger.
>> 
>> I added a corresponding assert and bailout in product to be on the safe side. The assert never triggered in my testing.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Typo2

src/hotspot/share/opto/compile.cpp line 3498:

> 3496:             assert(false, "Interface or abstract class pointers should not be compressed");
> 3497:           } else {
> 3498:             new_in2 = ConNode::make(t->make_narrowklass());

When I was looking through this code, I was hoping there'd be some sort of assert in the make_narrowklass function so any caller would assert but maybe you don't have that info?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21784#discussion_r1822503328