RFR: AArch64 remaining CAS-obj impls

Andrew Haley aph at redhat.com
Thu Nov 17 12:35:44 UTC 2016


On 17/11/16 12:08, Roman Kennke wrote:
> Am Donnerstag, den 17.11.2016, 11:48 +0000 schrieb Andrew Haley:
>> On 17/11/16 10:07, Roman Kennke wrote:
>>>
>>> I found one more bug in my implementation. For
>>> compareAndExchangeN, we need to restore the uncompressed old-value
>>> into the result register before leaving.
>>>
>>> http://cr.openjdk.java.net/~rkennke/aarch64-cas/webrev.01/
>>>
>>> Ok now?
>>
>> What does this mean?
>>
>> +        __ cmpxchg_oop_shenandoah(addr, cmpval, newval,
>> Assembler::word, true, true, false);
>>
>> No cheating by looking at the declaration of cmpxchg_oop_shenandoah!
> 
> ;-)
> 
> I know, it is not the crytal clear diamond of software engineering :-)
> 
> In my defense, it's mostly adopted from existing code, and at least
> it's consistent.
> 
> I agree it should be refactored, and if we go and do that, then for all
> of the similar cmpxchg() methods in MacroAssembler.

The maxims "tidy up as you go" and "don't make it any worse" apply
here.  You've added another boolean; surely the least you can do is
comment it at the call site.

Also, it's hard to understand what this last bit does:

2242   cmp(result, tmp2);
2243   // Retry with expected now being the value we just loaded from addr.
2244   br(Assembler::EQ, retry);
2245   if (size == word && res != noreg) {
2246     // For cmp-and-exchange and narrow oops, we need to restore
2247     // the compressed old-value.
2248     mov(result, expected);
2249   }

I think it's correct because expected really contains the result that
was read.  But heavens, this is confusing code.

Andrew.


More information about the shenandoah-dev mailing list