RFR: 8346836: C2: Introduce a way to verify the correctness of ConstraintCastNodes at runtime [v7]

[Vladimir Ivanov]

On Tue, 8 Apr 2025 20:08:44 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 860:
>> 
>>> 858:   movl(c_rarg1, type->_lo);
>>> 859:   movl(c_rarg2, type->_hi);
>>> 860:   call(RuntimeAddress(CAST_FROM_FN_PTR(address, abort_checked_cast_int)));
>> 
>> Do you need to align stack pointer before the call? It may be the reason why stack traces aren't printed.
>
> I believe C2 stacks are 16-byte aligned so we should not need to manually do the alignment?

Indeed. Still puzzled why stack traces are omitted. Do you have a reproducer to try?

>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.hpp line 47:
>> 
>>> 45:   void fast_unlock_lightweight(Register obj, Register reg_rax, Register t, Register thread);
>>> 46: 
>>> 47:   void checked_cast_int(const TypeInt* type, Register val);
>> 
>> There's already some ambiguity around what "checked cast" really means (think of "CastPP" vs "CheckCastPP" vs "checkcast"). Let's not make it worse. I suggest to just name it as `verify_int[_in]_range`/`verify_long[_in]_range`.
>> 
>> Also, unpacking `type` (and pass low/upper bounds as arguments) would make code on receiving end a bit simpler.
>> 
>> And some invariants to assert: 
>>   * no constants (lo == hi);
>>   * no empty ranges (lo > hi).
>
> Thanks for the suggestions, I have changed this to `verify_int_in_range`. Expanding the `Type` before passing into the function would be really bad after #17508, though.

> Expanding the Type before passing into the function would be really bad after https://github.com/openjdk/jdk/pull/17508, though.

Can you elaborate, please? Are you saying that after #17508 there'll be more invariants to verify, so better to pass all the data encapsulated in a `Type`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22880#discussion_r2034058684
PR Review Comment: https://git.openjdk.org/jdk/pull/22880#discussion_r2034056637