Patchwork Fix bt[lq] related miscompilation (PR target/48774)

login
register
mail settings
Submitter Jakub Jelinek
Date May 2, 2011, 8:16 p.m.
Message ID <20110502201652.GW17079@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/93709/
State New
Headers show

Comments

Jakub Jelinek - May 2, 2011, 8:16 p.m.
Hi!

As written in the PR, the testcase in the patch is miscompiled on
x86_64-linux, because during IRA a *btdi operand is changed from
register to CONST_INT 1 (to which that register was initialized).
Unfortunately when both 2nd and 3rd ZERO_EXTEND operands are constant
integers, *testqi_ext_3{,_rex64} patterns match it wrongly too,
but of course when splitting it into andl/q (which isn't desirable on
TARGET_USE_BT targets anyway) using CCCmode is wrong, as the jump then
looks at the carry bit set by bt{l,q}, while after and{l,q} the interesting
bit is the zero flag.

The following patch is one of the many possibilities to fix it,
bootstrapped/regtested on x86_64-linux and i686-linux.

Other options include just doing != CCCmode tests and remove the const1_rtx
checks from the patch, or on the other side also testing TARGET_USE_BT ||
-Os, or for the define_insn moving *bt<mode> pattern earlier (the splitter
would still need guarding), or perhaps representing bt{l,q} insns

	Jakub

Patch

differently in RTL.  I believe this bug is latent starting with 4.4,
when *bt<mode> insn has been added.

2011-05-02  Jakub Jelinek  <jakub@redhat.com>

	PR target/48774
	* config/i386/i386.md (testqi_ext_3_rex64, textqi_ext_3,
	split after testqi_ext_3*): Don't match CCCmode set with
	const1_rtx as last ZERO_EXTRACT operand.

	* gcc.dg/pr48774.c: New test.

--- gcc/config/i386/i386.md.jj	2011-05-02 18:39:23.000000000 +0200
+++ gcc/config/i386/i386.md	2011-05-02 19:09:23.000000000 +0200
@@ -7756,6 +7756,8 @@  (define_insn "*testqi_ext_3_rex64"
 		 (const_int 0)))]
   "TARGET_64BIT
    && ix86_match_ccmode (insn, CCNOmode)
+   && (GET_MODE (SET_DEST (PATTERN (insn))) != CCCmode
+       || operands[2] != const1_rtx)
    && INTVAL (operands[1]) > 0
    && INTVAL (operands[2]) >= 0
    /* Ensure that resulting mask is zero or sign extended operand.  */
@@ -7777,6 +7779,8 @@  (define_insn "*testqi_ext_3"
 		   (match_operand:SI 2 "const_int_operand" ""))
 		 (const_int 0)))]
   "ix86_match_ccmode (insn, CCNOmode)
+   && (GET_MODE (SET_DEST (PATTERN (insn))) != CCCmode
+       || operands[2] != const1_rtx)
    && INTVAL (operands[1]) > 0
    && INTVAL (operands[2]) >= 0
    && INTVAL (operands[1]) + INTVAL (operands[2]) <= 32
@@ -7794,7 +7798,8 @@  (define_split
 	     (match_operand 3 "const_int_operand" "")
 	     (match_operand 4 "const_int_operand" ""))
 	   (const_int 0)]))]
-  "ix86_match_ccmode (insn, CCNOmode)"
+  "ix86_match_ccmode (insn, CCNOmode)
+   && (GET_MODE (operands[0]) != CCCmode || operands[4] != const1_rtx)"
   [(set (match_dup 0) (match_op_dup 1 [(match_dup 2) (const_int 0)]))]
 {
   rtx val = operands[2];
--- gcc/testsuite/gcc.dg/pr48774.c.jj	2011-05-02 19:12:32.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr48774.c	2011-05-02 11:03:55.000000000 +0200
@@ -0,0 +1,38 @@ 
+/* PR target/48774 */
+/* { dg-do run } */
+/* { dg-options "-O2 -funroll-loops" } */
+
+extern void abort (void);
+unsigned long int s[24]
+  = { 12, ~1, 12, ~2, 12, ~4, 12, ~8, 12, ~16, 12, ~32,
+      12, ~64, 12, ~128, 12, ~256, 12, ~512, 12, ~1024, 12, ~2048 };
+struct { int n; unsigned long *e[12]; } g
+  = { 12, { &s[0], &s[2], &s[4], &s[6], &s[8], &s[10], &s[12], &s[14],
+	    &s[16], &s[18], &s[20], &s[22] } };
+int c[12];
+
+__attribute__((noinline, noclone)) void
+foo (void)
+{
+  int i, j;
+  for (i = 0; i < g.n; i++)
+    for (j = 0; j < g.n; j++)
+      {
+	if (i == j && j < g.e[0][0] && (g.e[i][1] & (1UL << j)))
+	  abort ();
+	if (j < g.e[0][0] && (g.e[i][1] & (1UL << j)))
+	  c[i]++;
+      }
+}
+
+int
+main ()
+{
+  int i;
+  asm volatile ("" : "+m" (s), "+m" (g), "+m" (c));
+  foo ();
+  for (i = 0; i < 12; i++)
+    if (c[i] != 11)
+      abort ();
+  return 0;
+}