[lworld] RFR: 8372700: [lworld] compiler/c2/irTests/stable/* fail with --enable-preview [v3]

[Tobias Hartmann]

On Fri, 19 Dec 2025 12:03:47 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> src/hotspot/share/ci/ciFlatArray.cpp line 47:
>> 
>>> 45: }
>>> 46: 
>>> 47: ciConstant ciFlatArray::check_constant_null_marker_cache(int off) {
>> 
>> Do we really need a cache here?
>
> Maybe? I read
> https://github.com/openjdk/valhalla/blob/69399cedf6fe208832a66c134d370af860154bc2/src/hotspot/share/ci/ciObject.hpp#L62-L63
> and
> https://github.com/openjdk/valhalla/blob/69399cedf6fe208832a66c134d370af860154bc2/src/hotspot/share/ci/ciObject.cpp#L173-L175
> 
> It seems to be more a correctness thing than a performance matter. It couldn't see why I wouldn't have a similar risk.

Ah, right, that makes sense!

>> src/hotspot/share/ci/ciInstance.cpp line 106:
>> 
>>> 104: // Constant value of the null marker.
>>> 105: ciConstant ciInstance::null_marker_value() {
>>> 106:   if (!klass()->is_inlinetype()) {
>> 
>> Should this be an assert?
>
> Matter of point of view. I prefer to have the semantics
> 
> if inlinetype => get the null marker
> else return invalid constant (since it doesn't make sense).
> 
> rather than
> 
> check is inline type
> get null marker
> 
> At call site, in the first case, I need to
> 
> nm = inst->null_marker_value();
> nm is true? (might be false or invalid)
> 
> in the second, I need to do
> 
> is inst an inline type?
> if so, nm = inst->null_marker_value();
> nm is true? (might be false, but not invalid)
> 
> 
> I prefer the first pattern, it's rather safer, and I think a lot of the code involving ciConstant have this behavior of "you get the constant if I can, otherwise, you get invalid".

Okay, fine with me!

>> test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java line 82:
>> 
>>> 80:     @IR(counts = { IRNode.LOAD, ">0" })
>>> 81:     @IR(applyIf = {"enable-valhalla", "false"}, failOn = { IRNode.MEMBAR })
>>> 82:     @IR(applyIf = {"enable-valhalla", "true"}, counts = { IRNode.MEMBAR, ">0" })
>> 
>> This needs a comment explaining why barriers are sometimes observed.
>
> I agree. The problem is that you told me it's expected, but I forgot the reason.

Did you remember what the problem was? :)

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1826#discussion_r2665042458
PR Review Comment: https://git.openjdk.org/valhalla/pull/1826#discussion_r2665038338
PR Review Comment: https://git.openjdk.org/valhalla/pull/1826#discussion_r2665034327