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

[Rich Felker <dalias@libc.org>]

On Tue, Mar 29, 2016 at 03:54:02AM +0000, Jaydeep Patil wrote:
> >-----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
> 
> This is what compiler has generated for above function:
> 
> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
> 
> Objdump:
> 
> 00000000 <ll1k>:
>    0:   24821000        addiu   v0,a0,4096
>    4:   7c420036        ll      v0,0(v0)
>    8:   d81f0000        jrc     ra
>    c:   00000000        nop

Can you try gcc -S instead of -c (still at -O3) to produce asm output
without assembling it?

Rich