[PATCH] __up_read and gcc-3.0

[PATCH] __up_read and gcc-3.0

[Petr Vandrovec]

Hi Alan,
  can you apply this patch to next 2.4.4-acX ? This fixes problem with
gcc3.0 (20010426) unable to compile this under some conditions. As
__up_write() uses same code ("i".... instead of tmp variable), I think
that you should apply this. It can cause slower code, as gcc cannot
move "movl -RWSEM_ACTIVE_READ_BIAS,%edx" away from "xadd" anymore,
but as "lock xadd" is slow anyway, it should not matter.

  I looked at generated code in cases where it originally failed, and
generated code looks OK to me.
					Thanks,
						Petr Vandrovec
						vandrove@vc.cvut.cz


diff -urdN linux/include/asm-i386/rwsem.h linux/include/asm-i386/rwsem.h
--- linux/include/asm-i386/rwsem.h	Fri Apr 27 22:48:24 2001
+++ linux/include/asm-i386/rwsem.h	Wed May  9 16:31:57 2001
@@ -148,9 +148,9 @@
  */
 static inline void __up_read(struct rw_semaphore *sem)
 {
-	__s32 tmp = -RWSEM_ACTIVE_READ_BIAS;
 	__asm__ __volatile__(
 		"# beginning __up_read\n\t"
+		"  movl      %2,%%edx\n\t"
 LOCK_PREFIX	"  xadd      %%edx,(%%eax)\n\t" /* subtracts 1, returns the old value */
 		"  js        2f\n\t" /* jump if the lock is being waited upon */
 		"1:\n\t"
@@ -164,9 +164,9 @@
 		"  jmp       1b\n"
 		".previous\n"
 		"# ending __up_read\n"
-		: "+m"(sem->count), "+d"(tmp)
-		: "a"(sem)
-		: "memory", "cc");
+		: "+m"(sem->count)
+		: "a"(sem), "i"(-RWSEM_ACTIVE_READ_BIAS)
+		: "memory", "cc", "edx");
 }
 
 /*

Re: [PATCH] __up_read and gcc-3.0

[Tom Leete]

Petr Vandrovec wrote:
> 
> Hi Alan,
>   can you apply this patch to next 2.4.4-acX ? This fixes problem with
> gcc3.0 (20010426) unable to compile this under some conditions. As
> __up_write() uses same code ("i".... instead of tmp variable), I think
> that you should apply this. It can cause slower code, as gcc cannot
> move "movl -RWSEM_ACTIVE_READ_BIAS,%edx" away from "xadd" anymore,
> but as "lock xadd" is slow anyway, it should not matter.
> 
>   I looked at generated code in cases where it originally failed, and
> generated code looks OK to me.
>                                         Thanks,
>                                                 Petr Vandrovec
>                                                 vandrove@vc.cvut.cz
> 
> diff -urdN linux/include/asm-i386/rwsem.h linux/include/asm-i386/rwsem.h
> --- linux/include/asm-i386/rwsem.h      Fri Apr 27 22:48:24 2001
> +++ linux/include/asm-i386/rwsem.h      Wed May  9 16:31:57 2001
> @@ -148,9 +148,9 @@
>   */
>  static inline void __up_read(struct rw_semaphore *sem)
>  {
> -       __s32 tmp = -RWSEM_ACTIVE_READ_BIAS;
>         __asm__ __volatile__(
>                 "# beginning __up_read\n\t"
> +               "  movl      %2,%%edx\n\t"
>  LOCK_PREFIX    "  xadd      %%edx,(%%eax)\n\t" /* subtracts 1, returns the old value */
>                 "  js        2f\n\t" /* jump if the lock is being waited upon */
>                 "1:\n\t"
> @@ -164,9 +164,9 @@
>                 "  jmp       1b\n"
>                 ".previous\n"
>                 "# ending __up_read\n"
> -               : "+m"(sem->count), "+d"(tmp)
> -               : "a"(sem)
> -               : "memory", "cc");
> +               : "+m"(sem->count)
> +               : "a"(sem), "i"(-RWSEM_ACTIVE_READ_BIAS)
> +               : "memory", "cc", "edx");
>  }
> 
>  /*

Hi Petr,

My solution to this was to relax +d(tmp) to +m(tmp). Posted a few days ago.
I have larger problems with 2.4.5-pre1 and have not gone back to check what
comes out. Being a product of pure reason (and not much of that), mine
deserves suspicion.

Cheers,
Tom

-- 
The Daemons lurk and are dumb. -- Emerson

Re: [PATCH] __up_read and gcc-3.0

[Petr Vandrovec]

On 11 May 01 at 9:13, Tom Leete wrote:
> >         __asm__ __volatile__(
> >                 "# beginning __up_read\n\t"
> > +               "  movl      %2,%%edx\n\t"
> >  LOCK_PREFIX    "  xadd      %%edx,(%%eax)\n\t" /* subtracts 1, returns the old value */
> >                 "  js        2f\n\t" /* jump if the lock is being waited upon */
> >                 "1:\n\t"
> 
> My solution to this was to relax +d(tmp) to +m(tmp). Posted a few days ago.
> I have larger problems with 2.4.5-pre1 and have not gone back to check what
> comes out. Being a product of pure reason (and not much of that), mine
> deserves suspicion.

Changing +d => +m could generate 'xadd (%%xyz),(%%eax)' which does not exist.
Maybe +r, but in this case do not forget to add push/pop %%edx around
call to rwsem_wake. Otherwise you can have some corruption if gcc decides
to use %edx for some local variable around __up_read.
                                                    Best regards,
                                                        Petr Vandrovec
                                                        vandrove@vc.cvut.cz