RFR: JDK-8213371: GC/C2 abstraction and cleanup to handle custom offset for GC memory accesses

[Roman Kennke]

Hi Vladimir,

> 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:
>>>
>>> http://hg.openjdk.java.net/jdk/jdk/file/0e8084c8cbb7/src/hotspot/share/opto/memnode.cpp#l1737
>>>
>>
>> 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:
>>
>> Incremental:
>> http://cr.openjdk.java.net/~rkennke/JDK-8213371/webrev.01.diff/
>> Full:
>> http://cr.openjdk.java.net/~rkennke/JDK-8213371/webrev.01/
> 
> Good.

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
> account.
> 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!

Roman


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181106/e61305c1/signature.asc>