[PATCH][4.0] Wrong code when rotate is expanded

Andreas Krebbel krebbel1@de.ibm.com
Thu Sep 22 11:20:00 GMT 2005


Hello,

the attached testcase failes on S/390 when compiled with gcc 4.0.x
under -O1 but not with -O0.

The problem occurs when the following tree expression is expanded:

  B = B + A + *D.1182 <<< ??? >>> ((int) (B + A) & 31);

??? stands for a rotate left node

B + A + *D.1182 is the operand to be rotated and (B + A) & 31 the
rotation count. When this is splitted into two shifts the result
of the first shift is stored in B and therefore clobbers the
shift operand used for the second shift 
(which would be 32 - ((B + A) & 31)).

The same could potentially happen with gcc 4.1 but the testcase
passes here due to different tree optimization. The addition of
A and B is factored out beforehand:

  D.1299 = B + A;
  B = D.1299 + MEM[base: &L[0], index: D.1386] <<< ??? >>> ((int) D.1299 & 31);

The patch removes the target operand of the rotate as possible target
for the first shift and adds it as target for the second.

Bootstrapped on i686, s390 and s390x with gcc 4.0.2 and gcc 4.1.
No testsuite regressions for 4.0.2 and with 4.1 on i686 I see
a small diff (8 vanished/2 new) regarding libmudflap.cth/pass39-frag.c 
fails which seem to be caused by a race.

OK for 4.0.2 and/or mainline?

Bye,

-Andreas-

2005-09-22  Andreas Krebbel  <krebbel1@de.ibm.com>

	* expmed.c (expand_shift): Don't use the target of the rotate as
	target for the first expanded shift insn.
	* testsuite/gcc.dg/20050922-1.c: Testcase added.


Index: gcc/expmed.c
===================================================================
*** gcc/expmed.c.orig	2005-09-22 10:00:20.000000000 +0200
--- gcc/expmed.c	2005-09-22 10:53:47.000000000 +0200
*************** expand_shift (enum tree_code code, enum 
*** 2239,2247 ****
  	      shifted = force_reg (mode, shifted);
  
  	      temp = expand_shift (left ? LSHIFT_EXPR : RSHIFT_EXPR,
! 				   mode, shifted, new_amount, subtarget, 1);
  	      temp1 = expand_shift (left ? RSHIFT_EXPR : LSHIFT_EXPR,
! 				    mode, shifted, other_amount, 0, 1);
  	      return expand_binop (mode, ior_optab, temp, temp1, target,
  				   unsignedp, methods);
  	    }
--- 2239,2247 ----
  	      shifted = force_reg (mode, shifted);
  
  	      temp = expand_shift (left ? LSHIFT_EXPR : RSHIFT_EXPR,
! 				   mode, shifted, new_amount, 0, 1);
  	      temp1 = expand_shift (left ? RSHIFT_EXPR : LSHIFT_EXPR,
! 				    mode, shifted, other_amount, subtarget, 1);
  	      return expand_binop (mode, ior_optab, temp, temp1, target,
  				   unsignedp, methods);
  	    }
Index: gcc/testsuite/gcc.dg/20050922-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/20050922-1.c	2005-09-22 12:48:53.000000000 +0200
***************
*** 0 ****
--- 1,41 ----
+ /* This revealed a bug when rotates are expanded into
+    two shifts.  */
+ 
+ /* { dg-do run } */
+ /* { dg-options "-O1 -std=c99" } */
+ 
+ #include <stdint.h>
+ 
+ extern void abort (void);
+ 
+ uint32_t
+ f (uint32_t *S, int j)
+ {
+   uint32_t A, B, k, L[2] = {1234, 5678};
+   int i, m;
+ 
+   A = B = 0;
+   for (i = 0; i < j; i++)
+     {
+       k = (S[i] + A + B) & 0xffffffffL;
+       A = S[i] =
+       ((k << (3 & 0x1f)) | ((k & 0xffffffff) >> (32 - (3 & 0x1f)))); 
+ 
+       m = (int) (A + B);
+       k = (L[i] + A + B) & 0xffffffffL;
+       B = L[i] =
+ 	((k << (m & 0x1f)) | ((k & 0xffffffff) >> (32 - (m & 0x1f))));
+     }
+   return L[0] + L[1];
+ }
+ 
+ int
+ main ()
+ {
+   uint32_t S[2] = {0xffff, 0xffffff};
+ 
+   if (f (S,2)!= 1392607300)
+     abort();
+ 
+   return 0;
+ }



More information about the Gcc-patches mailing list