RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil <Jaydeep.Patil@imgtec.com>]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 28 March 2016 PM 06:35
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
>> >> >I was just saying it makes the code less cluttered to use them
>> >> >spuriously even though we don't need to:
>> >> >
>> >> >		".set push ; "
>> >> >#if __mips_isa_rev < 6
>> >> >		".set mips2 ; "
>> >> >#endif
>> >> >		"ll %0, %1 ; .set pop"
>> >> >
>> >> >or similar.
>> >> >
>> >> >It's also not clear to me whether the "m" constraint is valid
>> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
>> >> >offset now instead of a
>> >16-bit offset.
>> >> >The compiler could generate an address expression whose offset
>> >> >part does not fit in 9 bits. In that case we may need to #if the
>> >> >whole function (or at least the __asm__ statement) separately
>> >> >rather than just
>> >skipping the .set mips2....
>> >> >
>> >>
>> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
>> >
>> >How can you assume the offset will be 0? It's the compiler's choice
>> >what to use. For instance, a_cas(&foo->bar, t, s) is likely to have
>> >an offset equal to offsetof(__typeof__(foo),bar). AFAIK this happens
>> >in practice with small offsets in mutex structures, etc. so the bug
>> >may be unlikely to be hit, but I think it's still an incorrect-constraint bug.
>>
>> Compiler generates appropriate LL/SC based on the offset.
>> Compiler adds the offset to the base register if it does not fit 9bits.
>
>The compiler has no way of knowing that the operand will be used with ll with
>the 9-bit offset restriction; as far as it knows, it will be used in a normal
>context where a 16-bit offset is valid. I don't have a toolchain that will target
>r6, but you can try the following program which produces an offset of 4096 for
>loading p[1024]:
>
>unsigned ll1k(volatile unsigned *p)
>{
>	unsigned val;
>	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
>"memory" );
>	return val;
>}
>
>I would expect this to produce errors at assembly time on r6.
>
>Rich

With -O0 optimization level:

00000000 <ll1k>:
   0:   27bdffe8        addiu   sp,sp,-24
   4:   afbe0014        sw      s8,20(sp)
   8:   03a0f025        move    s8,sp
   c:   afc40018        sw      a0,24(s8)
  10:   8fc20018        lw      v0,24(s8)
  14:   24421000        addiu   v0,v0,4096
  18:   7c420036        ll      v0,0(v0)
  1c:   afc20008        sw      v0,8(s8)
  20:   8fc20008        lw      v0,8(s8)
  24:   03c0e825        move    sp,s8
  28:   8fbe0014        lw      s8,20(sp)
  2c:   27bd0018        addiu   sp,sp,24
  30:   d81f0000        jrc     ra