Subject: gcc 3.3.1 bug + patch
To: None <tech-toolchain@NetBSD.ORG>
From: Martin Husemann <martin@duskware.de>
List: tech-toolchain
Date: 08/15/2003 18:35:06
In xsrc/xfree/xc/programs/Xserver/cfb16 compiling cfbfillsp.c on sparc64
causes gcc 3.3.1 to emit an illegal instruction:

gcc -S -c -O2 -ansi -Dasm=__asm -I../../../programs/Xserver/cfb -I../../../programs/Xserver/mfb -I../../../programs/Xserver/mi -I../../../programs/Xserver/cfb24 -I../../../programs/Xserver/include -I../../../exports/include/X11 -I../../../include/fonts -I../../../include/extensions -I../../../programs/Xserver/Xext -I../../.. -I../../../exports/include -DCSRG_BASED -DSHAPE -DXKB -DLBX -DXAPPGROUP -DXCSECURITY -DTOGCUP -DXF86BIGFONT -DDPMSExtension -DPIXPRIV -DPANORAMIX -DRENDER -DRANDR -DAVOID_GLYPHBLT -DPIXPRIV -DSINGLEDEPTH -DXvExtension -DXFree86Server -DXvMCExtension -DSMART_SCHEDULE -DBUILDDEBUG -DXResExtension -DX_BYTE_ORDER=X_BIG_ENDIAN -D_XSERVER64 -DNDEBUG -DFUNCPROTO=15 -DNARROWPROTO -DPSZ=16 -DXFREE86 cfbfillsp.c

The assembler complains:
cfbfillsp.s:1005: Error: relocation overflow

and it is right, the instruction is:

	sll     %o5, 32, %g5

The "sll" on sparc is limited to a shift count < 32; for bigger values sllx
has to be used.

I did a small change to sparc.md that seems to avoid it, and while there did
the same for all similar instructions (srl and sra). I assume if compiling for
32bit targets gcc somehow knows that a register can not be shifted more than
31bit - so there is no need to protect the "x" variants of the instruction for
sparc targets. Is that assumption correct? (If not, I'd have no idea how to
deal with that)

So, is this patch OK? If yes, could someone check this change with the gcc
people? I could not find a coresponding post-3.3.1 change in sparc.md, but I
didn't check the splitted *.md files in gcc-current.

Martin
P.S.: I did the sra/srl changes more or less mechanicaly, they should be
reviewed by a sparc person for the subtle differences between the 32 and
64 bit versions (which do not exist for sll/sllx, IIRC).

Index: sparc.md
===================================================================
RCS file: /cvsroot/src/gnu/dist/gcc/gcc/config/sparc/sparc.md,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 sparc.md
--- sparc.md	2003/07/23 02:41:59	1.1.1.1
+++ sparc.md	2003/08/15 16:04:35
@@ -6881,7 +6881,14 @@
 {
   if (operands[2] == const1_rtx)
     return "add\t%1, %1, %0";
-  return "sll\t%1, %2, %0";
+  if ((GET_CODE (operands[2]) == CONST_INT
+     && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) < 32)
+   || (GET_CODE (operands[2]) == CONST_DOUBLE
+     && CONST_DOUBLE_HIGH (operands[2]) == 0
+      && (unsigned HOST_WIDE_INT) CONST_DOUBLE_LOW (operands[2]) < 32))
+    return "sll\t%1, %2, %0";
+  else
+    return "sllx\t%1, %2, %0";
 }
   [(set (attr "type")
 	(if_then_else (match_operand 2 "const1_operand" "")
@@ -6969,7 +6976,16 @@
 	(ashiftrt:SI (match_operand:SI 1 "register_operand" "r")
 		     (match_operand:SI 2 "arith_operand" "rI")))]
   ""
-  "sra\t%1, %2, %0"
+{
+  if ((GET_CODE (operands[2]) == CONST_INT
+     && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) < 32)
+   || (GET_CODE (operands[2]) == CONST_DOUBLE
+     && CONST_DOUBLE_HIGH (operands[2]) == 0
+      && (unsigned HOST_WIDE_INT) CONST_DOUBLE_LOW (operands[2]) < 32))
+    return "sra\t%1, %2, %0";
+  else
+    return "srax\t%1, %2, %0";
+}
   [(set_attr "type" "shift")])
 
 (define_insn "*ashrsi3_extend"
@@ -6977,7 +6993,16 @@
 	(sign_extend:DI (ashiftrt:SI (match_operand:SI 1 "register_operand" "r")
 				     (match_operand:SI 2 "arith_operand" "r"))))]
   "TARGET_ARCH64"
-  "sra\t%1, %2, %0"
+{
+  if ((GET_CODE (operands[2]) == CONST_INT
+     && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) < 32)
+   || (GET_CODE (operands[2]) == CONST_DOUBLE
+     && CONST_DOUBLE_HIGH (operands[2]) == 0
+      && (unsigned HOST_WIDE_INT) CONST_DOUBLE_LOW (operands[2]) < 32))
+    return "sra\t%1, %2, %0";
+  else
+    return "srax\t%1, %2, %0";
+}
   [(set_attr "type" "shift")])
 
 ;; This handles the case as above, but with constant shift instead of
@@ -7040,7 +7065,16 @@
 	(lshiftrt:SI (match_operand:SI 1 "register_operand" "r")
 		     (match_operand:SI 2 "arith_operand" "rI")))]
   ""
-  "srl\t%1, %2, %0"
+{
+  if ((GET_CODE (operands[2]) == CONST_INT
+     && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) < 32)
+   || (GET_CODE (operands[2]) == CONST_DOUBLE
+     && CONST_DOUBLE_HIGH (operands[2]) == 0
+      && (unsigned HOST_WIDE_INT) CONST_DOUBLE_LOW (operands[2]) < 32))
+    return "srl\t%1, %2, %0";
+  else
+    return "srlx\t%1, %2, %0";
+}
   [(set_attr "type" "shift")])
 
 ;; This handles the case where