diff mbox

pr53138 - miscompilation of spaceship operator

Message ID 1335520168-28993-1-git-send-email-bonzini@gnu.org
State New
Headers show

Commit Message

Paolo Bonzini April 27, 2012, 9:49 a.m. UTC 
The testcase is miscompiled to this:

	...
	movl	(%rsi), %ecx
	cmpl	%ecx, (%rdi)
	sbbl	%edx, %edx
	cmovbe	%edx, %eax
	ret

but sbbl only preserves the carry flag, not the zero flag.  I suppose
one could use different patterns with different CC modes, but this is
the safest fix.

Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?

I also reproduced this at least on 4.7; help would be welcome on
committing it to the branches.  Thanks in advance!

Paolo

2012-04-27  Paolo Bonzini  <bonzini@gnu.org>

	PR target/53138
	* config/i386/i386.md (x86_mov<mode>cc_0_m1_neg): Add clobber.

2012-04-27  Paolo Bonzini  <bonzini@gnu.org>

	PR target/53138
	* gcc.c-torture/execute/20120427-1.c: New testcase.

Comments

Uros Bizjak April 27, 2012, 10:03 a.m. UTC | #1 
On Fri, Apr 27, 2012 at 11:49 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> The testcase is miscompiled to this:
>
>        ...
>        movl    (%rsi), %ecx
>        cmpl    %ecx, (%rdi)
>        sbbl    %edx, %edx
>        cmovbe  %edx, %eax
>        ret
>
> but sbbl only preserves the carry flag, not the zero flag.  I suppose
> one could use different patterns with different CC modes, but this is
> the safest fix.
>
> Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?

OK.

> I also reproduced this at least on 4.7; help would be welcome on
> committing it to the branches.  Thanks in advance!

I can take care of this, but I won' be able to do this until Monday.

Thanks,
Uros.
diff mbox

Patch

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revisione 186859)
+++ config/i386/i386.md	(copia locale)
@@ -16439,7 +16439,8 @@ 
 (define_insn "*x86_mov<mode>cc_0_m1_neg"
   [(set (match_operand:SWI48 0 "register_operand" "=r")
 	(neg:SWI48 (match_operator 1 "ix86_carry_flag_operator"
-		    [(reg FLAGS_REG) (const_int 0)])))]
+		    [(reg FLAGS_REG) (const_int 0)])))
+   (clobber (reg:CC FLAGS_REG))]
   ""
   "sbb{<imodesuffix>}\t%0, %0"
   [(set_attr "type" "alu")
Index: testsuite/gcc.c-torture/execute/20120427-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/20120427-1.c	(revisione 0)
+++ testsuite/gcc.c-torture/execute/20120427-1.c	(revisione 0)
@@ -0,0 +1,36 @@ 
+typedef struct sreal
+{
+  unsigned sig;		/* Significant.  */
+  int exp;		/* Exponent.  */
+} sreal;
+
+sreal_compare (sreal *a, sreal *b)
+{
+  if (a->exp > b->exp)
+    return 1;
+  if (a->exp < b->exp)
+    return -1;
+  if (a->sig > b->sig)
+    return 1;
+  return -(a->sig < b->sig);
+}
+
+sreal a[] = {
+   { 0, 0 },
+   { 1, 0 },
+   { 0, 1 },
+   { 1, 1 }
+};
+
+int main()
+{
+  int i, j;
+  for (i = 0; i <= 3; i++) {
+    for (j = 0; j < 3; j++) {
+      if (i < j && sreal_compare(&a[i], &a[j]) != -1) abort();
+      if (i == j && sreal_compare(&a[i], &a[j]) != 0) abort();
+      if (i > j && sreal_compare(&a[i], &a[j]) != 1) abort();
+    }
+  }
+  return 0;
+}