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