RFR: JDK-8203157: Object equals abstraction for BarrierSetAssembler

[Erik Österlund]

Hi Roman,

Sure. As long as there is no need for AS_RAW equals in the assembly 
code, we don't need to add it now.
However, that means that there are currently no properties in the 
decorators we care about at the moment. Therefore, the decorator 
parameter of obj_equals should be removed; it serves no purpose.

Thanks,
/Erik

On 2018-06-05 17:01, Roman Kennke wrote:
> Am 05.06.2018 um 17:00 schrieb 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.
> Yes. That is correct.
>
>>> 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.
> I don't lean very much either way. But it should be discussed under
> JDK-8203232. Considering that there is no use for cmpoop_raw() except as
> helper for BSA, do you agree that we don't need the AS_RAW hardwiring
> for obj_equals() ? Can I consider this patch Reviewed?
>
> Thanks, Roman
>