On Fri, 19 Dec 2025 08:59:35 GMT, Tobias Hartmann <[email protected]> wrote:

>> Flat accesses to a stable value can be expanded in a non-atomic way if the 
>> stable field is already initialized since they are read-only at this point. 
>> That allows to make more optimizations, and in particular to replace the 
>> access by a constant if it's known at compilation time.
>> 
>> There are 2 cases. First, flat stable non-array field. In this case, the 
>> value is known to be stable if the value is non-null, that is if the 
>> null-marker of the said field is 1. If we can find that the null marker has 
>> a constant value that is non zero, we expand non-atomically. That is done by 
>> finding the field we are trying to get from the offset. From the field, we 
>> can find the offset of the null marker, and then the null marker `ciField`, 
>> allowing to fetch its value if the holder is known to be a constant oop.
>> 
>> The other case is stable flat array. In this case, we need to find index of 
>> the containing element of the array, then with the offset, we can find the 
>> field we are trying to get. Finding the null marker here is a bit more 
>> tricky. Let's say we have
>> 
>> value class MyValue {
>>     int x;
>> }
>> class C {
>>     MyValue v;  // assumed flat.
>> }
>> 
>> the null marker for `v` is somewhat a field of `C`, as well as `v.x`. So I 
>> can just use `field_value` to get the null marker. But in `MyValue[]`, there 
>> isn't a single field for the null marker, but one "field" for each cell of 
>> the array, and there isn't a nice containing type in which it lives. The 
>> only way to get each piece of the array is by index (or offset). So, I 
>> needed specialized functions to access the null marker of a cell given the 
>> index/offset.
>> 
>> I also had to implement of bit of accessors for `ciFlatArray`. First, 
>> implement `element_value_by_offset` in `ciFlatArray` since the 
>> implementation of `ciArray` (super-class) was used, which computes the index 
>> from the provided offset, assuming a size of elements that doesn't take 
>> flattening into account as it used only the basic type, and not the layout 
>> helper. But also helpers to get a field of the flattened value class in a 
>> cell, to allow constant folding to get the constant value in an array.
>> 
>> The last part of the puzzle, necessary to make constant folding happen (see 
>> `Type::make_constant_from_field`), is to say that a field of a flattened 
>> inline type field is constant if the containing field if constant. In the 
>> words of the previous example, that means `x` is constant in `C` if `v` is 
>> strict and final (already there), or if `v` is constant itself. That ma...
>
> 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".

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1826#discussion_r2634847722

Reply via email to