RFR: 8296448: RISC-V: Fix temp usages of heapbase register killed by MacroAssembler::en/decode_klass_not_null [v2]

[Xiaolin Zheng]

On Thu, 17 Oct 2024 09:09:42 GMT, Andreas Schwab <duke at openjdk.org> wrote:

> There is a potential conflict with the use of t0 in decode_klass_not_null:
> 
> ```
> assert_different_registers(t0, xbase);
> ```
> 
> since tmp in load_klass defaults to t0.

Thank you for pointing this out. Yes, code here in `MacroAssembler::decode_klass_not_null` seems to have a potential issue. But in reality I assume it is dead code.


// In MacroAssembler::decode_klass_not_null()
//  -> CompressedKlassPointers::base() is non-null also when reaching here
if (CompressedKlassPointers::shift() != 0) {
    assert(LogKlassAlignmentInBytes == CompressedKlassPointers::shift(), "decode alg wrong");
    assert_different_registers(t0, xbase);
    shadd(dst, src, xbase, t0, LogKlassAlignmentInBytes);
}

As per Thomas' clear comments here [1], `base > 0 && shift > 0` should not happen. Perhaps we can perform some cleanup here to remove unreachable code that causes confusion. Maybe something like https://github.com/zhengxiaolinX/jdk/commit/973a071e51d116909b9713e52850b4990c025183.

But it is untested due to a lack of resources, so waiting for someone kind to take it over.

BTW the current PR (11010) is still needed because the heapbase register should not be used as a temp register here.

Hope this can explain well.

Best,
Xiaolin


[1] https://github.com/openjdk/jdk/blob/4dcc7f3f2629e857b20f72e99189db8781aa65ff/src/hotspot/share/oops/compressedKlass.cpp#L103-L104

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

PR Comment: https://git.openjdk.org/jdk/pull/11010#issuecomment-2422874709