RFR: 8187227: __m68k_cmpxchg() is not being used correctly

[Andrew Haley]

On 19/11/17 12:25, David Holmes wrote:
> On 18/11/2017 10:15 PM, John Paul Adrian Glaubitz wrote:
>> On 11/18/2017 12:46 PM, Andrew Haley wrote:
>>>> Without seeing how that wrapper actually works, it is hard to know
>>>> whether what you are saying and your fix is correct. The description of
>>>> the CAS at [1] doesn't help without knowing how it actually gets used.
>>>
>>> I can't see the point of the loops.
>>
>> The loops are necessary because it's not guaranteed that the CAS instruction
>> will succeed at first attempt. You basically have to keep trying until it
>> succeeds. At least to my current knowledge.
> 
> But a CAS can fail. Only if it can fail spuriously (ie for a reason 
> other than *dest!=expected) should a loop be needed.
> 
>> The loops are already in the code, FWIW. I am just modifying the code because
>> the current code makes incorrect assumptions on how __m68k_cmpxchg() actually
>> works.
> 
> True the loops do exist. But I for one can't make much sense out this 
> code at present, so impossible to validate your changes. If the code is 
> as broken as you suggest why don't you see massive failures during testing?

For avoidance of doubt, this is what GCC does:


int m68k_sync_compare_and_swap(int newval,
                           volatile int *ptr,
                           int oldval) {
  return __sync_val_compare_and_swap (ptr, oldval, newval);
}

m68k_sync_compare_and_swap:
	move.l 4(%sp),%d1
	move.l 8(%sp),%a0
	move.l 12(%sp),%d0
	cas.l %d0,%d1,(%a0)
	seq %d1
	rts

You could do many things worse than simply using __sync_val_compare_and_swap
for Zero on 68k.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671