diff mbox

Fix *anddi_2 (PR target/59101)

Message ID 20131113181247.GV27813@tucnak.zalov.cz
State New
Headers show

Commit Message

Jakub Jelinek Nov. 13, 2013, 6:12 p.m. UTC 
On Wed, Nov 13, 2013 at 06:38:00PM +0100, Uros Bizjak wrote:
> > --- gcc/config/i386/i386.md.jj  2013-11-12 11:31:31.000000000 +0100
> > +++ gcc/config/i386/i386.md     2013-11-13 10:14:10.981609589 +0100
> > @@ -7978,7 +7978,12 @@ (define_insn "*anddi_2"
> >          (const_int 0)))
> >     (set (match_operand:DI 0 "nonimmediate_operand" "=r,r,rm")
> >         (and:DI (match_dup 1) (match_dup 2)))]
> > -  "TARGET_64BIT && ix86_match_ccmode (insn, CCNOmode)
> > +  "TARGET_64BIT
> > +   && ix86_match_ccmode (insn, CONST_INT_P (operands[2])
> > +                              && INTVAL (operands[2]) > 0
> > +                              && (INTVAL (operands[2])
> > +                                  & (HOST_WIDE_INT_1 << 31)) != 0
> > +                              ? CCZmode : CCNOmode)
> 
> "Z" constraint, a.k.a x86_64_zext_immediate_operand won't allow
> constants with high bits set.

But x86_64_immediate_operand will.

> It looks to me, we can use mode_signbit_p here, and simplify the expression to

No, because mode_signbit_p tests if the constant is equal to the
sign bit, while we need to test whether the sign bit is set.
For & 0x80000000 it is the same thing, but for & 0x80000001 it is not.

> ix86_match_ccmode (insn, mode_signbit_p (SImode, operands[2])
>                                         ? CCZmode : CCNOmode)

Even if mode_signbit_p did something different, that would pessimize:
long long
foo (long long a)
{
  long long b = a & 0xfffffffff0000000LL;
  if (b > 0)
    bar ();
  return b;
}
because then we can use *anddi_2 just fine:
#(insn 7 25 8 2 (parallel [
#            (set (reg:CCNO 17 flags)
#                (compare:CCNO (and:DI (reg/v:DI 3 bx [orig:87 b ] [87])
#                        (const_int -268435456 [0xfffffffff0000000]))
#                    (const_int 0 [0])))
#            (set (reg/v:DI 3 bx [orig:87 b ] [87])
#                (and:DI (reg/v:DI 3 bx [orig:87 b ] [87])
#                    (const_int -268435456 [0xfffffffff0000000])))
#        ]) pr59101-3.c:7 392 {*anddi_2}
#     (nil))
        andq    $-268435456, %rbx       # 7     *anddi_2/2      [length = 7]
#(jump_insn 8 7 9 2 (set (pc)
#        (if_then_else (le (reg:CCNO 17 flags)
#                (const_int 0 [0]))
#            (label_ref 12)
#            (pc))) pr59101-3.c:7 607 {*jcc_1}
#     (expr_list:REG_DEAD (reg:CCNO 17 flags)
#        (int_list:REG_BR_PROB 3666 (nil)))
# -> 12)
        jle     .L2     # 8     *jcc_1  [length = 2]

as we aren't using the first alternative (andl), but another one.

> Please also add comment, this issue is quite non-obvious.

So what about this version?  If x86_64_zext_immediate_operand
returns true for non-CONST_INT (CONST_DOUBLE won't happen for DImode
operand with 64-bit HWI), it might be SYMBOL_REF/LABEL_REF etc. and
it is IMHO better to just assume it might have bit 31 set.

2013-11-13  Jakub Jelinek  <jakub@redhat.com>

	PR target/59101
	* config/i386/i386.md (*anddi_2): Only allow CCZmode if
	operands[2] is x86_64_zext_immediate_operand that might
	have bit 31 set.

	* gcc.c-torture/execute/pr59101.c: New test.



	Jakub
diff mbox

Patch

--- gcc/config/i386/i386.md.jj	2013-11-13 18:32:48.586808734 +0100
+++ gcc/config/i386/i386.md	2013-11-13 19:07:05.648122323 +0100
@@ -7978,7 +7978,20 @@  (define_insn "*anddi_2"
 	 (const_int 0)))
    (set (match_operand:DI 0 "nonimmediate_operand" "=r,r,rm")
 	(and:DI (match_dup 1) (match_dup 2)))]
-  "TARGET_64BIT && ix86_match_ccmode (insn, CCNOmode)
+  "TARGET_64BIT
+   && ix86_match_ccmode (insn,
+			 /* If we are going to emit andl instead of andq,
+			    and the operands[2] constant might have the
+			    SImode sign bit set, make sure the sign flag isn't
+			    tested, because the instruction will set the sign
+			    flag based on bit 31 rather than bit 63.
+			    If it isn't CONST_INT, conservatively assume
+			    it might have bit 31 set.  */
+			 (x86_64_zext_immediate_operand (operands[2], VOIDmode)
+			  && (!CONST_INT_P (operands[2])
+			      || (INTVAL (operands[2])
+				  & (HOST_WIDE_INT_1 << 31)) != 0))
+			 ? CCZmode : CCNOmode)
    && ix86_binary_operator_ok (AND, DImode, operands)"
   "@
    and{l}\t{%k2, %k0|%k0, %k2}
--- gcc/testsuite/gcc.c-torture/execute/pr59101.c.jj	2013-11-13 18:49:04.922736108 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr59101.c	2013-11-13 18:49:04.922736108 +0100
@@ -0,0 +1,15 @@ 
+/* PR target/59101 */
+
+__attribute__((noinline, noclone)) int
+foo (int a)
+{
+  return (~a & 4102790424LL) > 0 | 6;
+}
+
+int
+main ()
+{
+  if (foo (0) != 7)
+    __builtin_abort ();
+  return 0;
+}