RFR: JDK-8203157: Object equals abstraction for BarrierSetAssembler

[Roman Kennke]

Am 12.06.2018 um 11:11 schrieb Andrew Haley:
> On 06/11/2018 08:17 PM, Roman Kennke wrote:
>> Am 11.06.2018 um 19:11 schrieb Andrew Haley:
>>> On 06/11/2018 04:56 PM, Andrew Haley wrote:
>>>> On 06/08/2018 09:17 PM, Roman Kennke wrote:
>>>>> Why is it better? And how would I do that? It sounds like a fairly
>>>>> complex undertaking for a special case. Notice that if the oop doesn't
>>>>> qualify as immediate operand (quite likely for an oop?) it used to be
>>>>> moved into rscratch1 anyway a few lines below.
>>>>
>>>> Sorry for the slow reply.  I'm looking now.
>>>
>>> OK.  The problem is that this is a very bad code smell:
>>>
>>>       case T_ARRAY:
>>>         jobject2reg(opr2->as_constant_ptr()->as_jobject(), rscratch1);
>>>         __ cmpoop(reg1, rscratch1);
>>>
>>> I can't tell that this is correct.  rscratch1 is used by assembler
>>> macros, and I don't know if some other GC (e.g. ZGC) might need to use
>>> rscratch1 inside cmpoop.  The risk here is obvious.  The Right Thing
>>> to do IMO is to generate a scratch register for pointer comparisons.
>>>
>>> Unless, I guess, we know that cmpoop never ever needs a scratch
>>> register for any forseeable garbage collector.
>>>
>>
>> I do know that Shenandoah does not require a tmp reg. I also do know
>> that no other collector currently needs equals-barriers at all.
> 
> So cmpoop() is literally useless.  It does nothing except add a layer
> of obfuscation in the name of some possible future collector.

The layer of abstraction is needed by Shenandoah. We need special
handling for comparing oops. It is certainly not useless. Or are we
talking about different issues?

>> I cannot see into the future. I prefer to be pragmatic and solve
>> existing problems.
> 
> Perhaps, but this change you're asking me to review doesn't solve a
> problem, it creates one.  This is how technical debt happens.
> 
>> How about I add a comment to the obj_equals() API that says 'don't
>> use tmp reg X, and if you really need to, push/pop it or let the
>> compiler generate one for you' ?
> 
> It's awkward, isn't it?  I know that this is the wrong way to solve
> the problem, but I'm as vulnerable to social pressure as anyone else.
> Perhaps I should give up and choose an easy life.  :-)

Haha. No, your review is very appreciated.

Thank you,
Roman