RFR: Refactor asm acmp

[Roman Kennke]

Am 18.07.2017 um 17:54 schrieb Roman Kennke:
> Am 18.07.2017 um 16:23 schrieb Aleksey Shipilev:
>> On 07/18/2017 04:18 PM, Roman Kennke wrote:
>>> This bugs me since a while. Wherever we do compare objects in assembly
>>> code (i.e. interpreter and C1), we use to write something like:
>>>
>>> __ cmpptr(o1, o2)
>>>
>>> oopDesc::bs()->asm_acmp_barrier(o1, o2)
>>>
>>>
>>> where the asm_acmp_barrier() would *expect* to be preceded by cmpptr,
>>> and pick up its condition flags, etc, and produce the same condition flags.
>>>
>>> It all seems brittle and not very obvious.
>>>
>>> I propose to change that to:
>>>
>>> __ cmp_objects(o1, o2);
>>>
>>> and call the acmp barrier from there. This makes consuming code much
>>> more obvious, and the call to the barrier set more contained.
>> The patch looks fine, but this makes me thinking if we should instead reuse
>> cmpoop(), by implementing it at _LP64:
>>
>> 2643 #ifdef _LP64
>> 2644           __ movoop(rscratch1, o);
>> 2645           __ cmp_objects(reg1, rscratch1);
>>
>> 2646 #else
>> 2647           __ cmpoop(reg1, c->as_jobject());
>> 2648 #endif // _LP64
>>
>> That would make a good upstream patch too :)
> Yes, but cmpoop() takes a jobject. We need a Register (or Address, see
> below).
>
> As discussed on IRC, I renamed cmp_objects() to cmpoopptr().
>
> I am also now providing a cmpoopptr() that takes a (Register, Address),
> similar to cmpptr(). This removes some clutter in
> c1_LIRAssembler_x86.cpp and thus minimizes our diff against upstream.
> Notice that I have not added a similar method to BarrierSet. I never
> really liked the assembly stuff in BarrierSet, and would rather remove
> it. In the future, Erik's BarrierSet refactoring will provide a cool way
> to abstract that stuff.
>
> http://cr.openjdk.java.net/~rkennke/refactor-acmp/webrev.01/
> <http://cr.openjdk.java.net/%7Erkennke/refactor-acmp/webrev.01/>

Oops, some dead stuff sneaked into shenandoahBarrierSet.hpp. Removed:

http://cr.openjdk.java.net/~rkennke/refactor-acmp/webrev.02/
<http://cr.openjdk.java.net/%7Erkennke/refactor-acmp/webrev.02/>

Roman
>
> Roman
>