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 <ja...@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.
--- 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;
+}
Jakub
