RFR: JDK-8203157: Object equals abstraction for BarrierSetAssembler

[Erik Österlund]

Hi Roman,

On 2018-06-05 16:16, Roman Kennke wrote:
> Am 04.06.2018 um 14:38 schrieb Erik Österlund:
>> Hi Roman,
>>
>>   42
>>   43   virtual void obj_equals(MacroAssembler* masm, DecoratorSet
>> decorators,
>>   44                           Register obj1, Register obj2);
>>   45
>>
>> I don't think we need to pass in any decorators here. Perhaps one day
>> there will be some important semantic property to deal with, but today I
>> do not think there are any properties we care about, except possibly
>> AS_RAW, but that would never propagate into the BarrierSetAssembler anyway.
>>
>> On that topic, I noticed that today we do the raw version of e.g.
>> load_heap_oop inside of the BarrierSetAssembler, and to use it you would
>> call load_heap_oop(AS_RAW). But the cmpoop stuff does it in a different
>> way (cmpoop_raw in the macro assembler). I think it would be ideal if we
>> could do it the same way, which would involve calling cmpoop with AS_RAW
>> to get a raw oop comparison, residing in BarrierSetAssembler, with the
>> usual hardwiring in the corresponding macro assembler function when it
>> observes AS_RAW.
>>
>> So it would look something like this:
>>
>> void cmpoop(Register src1, Address src2, DecoratorSet decorators =
>> AS_NORMAL);
>>
>> What do you think?
> cmpoop_raw() is not the AS_RAW base implementation. It's only there to
> help BarrierSetAssembler to implement the base
> obj_equals(Address|Register, jobject). We cannot access cmp_literal32()
> from outside the MacroAssembler.

In other words, there is no AS_RAW option "exposed" to public use, 
right? Maybe there is no need for raw equals in our assembly code.

> The mentioned hardwiring to call straight to BSA is probably going away too:
> https://bugs.openjdk.java.net/browse/JDK-8203232
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-May/032240.html

I'm not sure I'm convinced that is an improvement. The expected 
behaviour at the callsite is that the code in BarrierSetAssembler (which 
is the level of the hierarchy that implements raw accesses) is run, and 
nothing else. If anything else happens, it's a bug. So I hardwire that 
at the callsite to always match the expected behaviour. To instead let 
each level of the barrier class hierarchy remember to check for AS_RAW 
and delegate to the parent class in a way that ultimately has the exact 
same perceivable effect as the hardwiring, but in a much more error 
prone way, does not sound like an improvement to me. Perhaps I can be 
convinced otherwise if I understand what the concern is here and what 
problem we are trying to solve.

Thanks,
/Erik

> Thanks, Roman