Re: [PATCH RFC 19/67] MIPS: asm: atomic: Update asm and ISA constrains for MIPS R6 support

[Markos Chandras <Markos.Chandras@imgtec.com>]

On 12/18/2014 06:50 PM, David Daney wrote:
> On 12/18/2014 07:09 AM, Markos Chandras wrote:
>> MIPS R6 changed the opcodes for LL/SC instructions and reduced the
>> offset field to 9-bits. This has some undesired effects with the "m"
>> constrain since it implies a 16-bit immediate. As a result of which,
>> add a register ("r") constrain as well to make sure the entire address
>> is loaded to a register before the LL/SC operations. Also use macro
>> to set the appropriate ISA for the asm blocks
>>
> 
> Has support for MIPS R6 been added to GCC?
> 
> If so, that should include a proper constraint to be used with the new
> offset restrictions.  We should probably use that, instead of forcing to
> a "r" constraint.
> 
> 
>> Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>
>> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
>> ---
>>   arch/mips/include/asm/atomic.h | 50
>> +++++++++++++++++++++---------------------
>>   1 file changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/atomic.h
>> b/arch/mips/include/asm/atomic.h
>> index 6dd6bfc607e9..8669e0ec97e3 100644
>> --- a/arch/mips/include/asm/atomic.h
>> +++ b/arch/mips/include/asm/atomic.h
>> @@ -60,13 +60,13 @@ static __inline__ void atomic_##op(int i, atomic_t
>> * v)                \
>>                                           \
>>           do {                                \
>>               __asm__ __volatile__(                    \
>> -            "    .set    arch=r4000            \n"    \
>> -            "    ll    %0, %1        # atomic_" #op "\n"    \
>> +            "    .set    "MIPS_ISA_ARCH_LEVEL"        \n"    \
>> +            "    ll    %0, 0(%3)    # atomic_" #op "\n"    \
>>               "    " #asm_op " %0, %2            \n"    \
>> -            "    sc    %0, %1                \n"    \
>> +            "    sc    %0, 0(%3)            \n"    \
>>               "    .set    mips0                \n"    \
>>               : "=&r" (temp), "+m" (v->counter)            \
>> -            : "Ir" (i));                        \
>> +            : "Ir" (i), "r" (&v->counter));                \
> 
> You lost the "m" constraint, but are still modifying memory.  There is
> no "memory" clobber here, so we are no longer correctly describing what
> is happening.
> 
> 

Sorry I don't understand what you mean by  "you lost the "m"
constraint". +m (v->counter) is still there to denote that v->counter
memory is being modified no?

-- 
markos