RFR: JDK-8213371: GC/C2 abstraction and cleanup to handle custom offset for GC memory accesses
rkennke at redhat.com
Tue Nov 6 21:31:07 UTC 2018
> On 11/6/18 12:09 PM, Roman Kennke wrote:
>> Hi Vladimir,
>> Thanks for reviewing!
>>> Changes in compile.cpp and type.cpp look fine.
>>> I think in memnode.cpp (uint) casts are used to take into account
>>> OffsetTop and OffsetBot without explicitly checking for them. We don't
>>> expect OffsetTop here - there assert to check that. But 'off' could be
>>> OffsetBot. In line 1706 there is OffsetBot check but not in line 1737:
>> Ah! I was wondering. So by casting we implicitly accept OffsetBot as
>> off_beyond_header=true. Ok.
>>> I think you need to add explicit check OffsetBot for off_beyond_header.
>>> You did that in type.cpp.
>> Yes. However, I think it's clearer to include "|| off ==
>> Type::OffsetBot" in the second use of off_beyond_header, rather than
>> including it first, and then excluding it in the first use. Like this:
Thanks! Will push it through jdk/submit while waiting for a 2nd reviewer.
>> I must admit that I don't really understand the usefulness of including
>> OffsetBot in the 2nd clause, because that includes the case where offset
>> is not actually beyond the mark word, iow 'we don't know what the offset
>> is'. Right?
> Bot offset is common case for array access - we have to take it into
> Accesses to header should not be Bot (except LoadKlass as comment said -
> for which there are checks). Length and markword should have fixed offsets.
Ah ok. Thanks for the explanation! And for reviewing!
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: OpenPGP digital signature
More information about the hotspot-compiler-dev