This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Relax check against commuting XOR and ASHIFTRT in combine.c


combine.c includes a check which prevents

(ashiftrt (xor A C2) C1)

from being commuted to

(xor (ashiftrt A C1) (ashiftrt C2 C1))

for constants C1, C2 if C2 has its sign bit set.

Specifically, this prevents (ashiftrt (not A) C1) from being commuted to

(not (ashiftrt A C1))

because the former is rewritten to (ashiftrt (xor A -1) C1) above, with a comment /* Make this fit the case below. */ - which it no longer does.

If result_mode == shift_mode, I can see no reason to prevent this normalisation (indeed, I'm not sure I can see any reason to prevent it even if result_mode != shift_mode - but I've not managed to produce such a case in my own testing, as there are always intervening subreg's or sign_extend's, or to build a toolchain on which to reproduce the original bug, so I'm being cautious). Hence this patch allows commutation if the two modes are equal.

As an illustrative example, on AArch64, without this patch, compiling this snippet of the current arm_neon.h:

typedef long long int int64x1_t __attribute__ ((__vector_size__((8))));
int64x1 vcgez_s64(int64x1_t a)
{
  return (int64x1_t) {a >=0 ? -1 : 0};
}

produces a sequence involving a logical not (mvn) followed by asr (plus some inter-bank moves), as the combiner takes (ashiftrt (not x) 63), "simplifies", and fails to match the resulting RTL

(set (reg:DI 78 [ D.2593 ])
    (ashiftrt:DI (xor:DI (reg:DI 0 x0 [ aD.2565 ])
            (const_int -1 [0xffffffffffffffff]))
        (const_int 63 [0x3f])))

However, with this patch, the combiner simplifies to

(set (reg:DI 84 [ D.2604 ])
    (neg:DI (ge:DI (reg:DI 32 v0 [ aD.2569 ])
            (const_int 0 [0]))))

which matches a pattern to output the desired cmge instruction.

(For the curious: the check against commutation was added in January 2004, r76965, https://gcc.gnu.org/ml/gcc-patches/2004-01/msg03406.html - however, I've not been able to build an ADA toolchain of that era, or an Alpha/VMS toolchain, on which to reproduce the bug; if anyone has such and is willing and able to investigate, by all means!)

Testing:

aarch64-none-elf: check-gcc (cross-tested)
x86-none-linux-gnu: bootstrapped; check-gcc, check-ada, check-g++
arm-none-linux-gnueabi: bootstrapped
arm-none-eabi: check-gcc (cross-tested)


gcc/ChangeLog:

	* combine.c (simplify_shift_const_1): Allow commuting ASHIFTRT and XOR
	if same mode.

---------------------------------
diff --git a/gcc/combine.c b/gcc/combine.c
index 4e7ef55..29a9fc8 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10219,6 +10219,7 @@ simplify_shift_const_1 (enum rtx_code code, enum machine
              /* We can't do this if we have (ashiftrt (xor))  and the
                 constant has its sign bit set in shift_mode.  */
              && !(code == ASHIFTRT && GET_CODE (varop) == XOR
+                  && result_mode != shift_mode
                   && 0 > trunc_int_for_mode (INTVAL (XEXP (varop, 1)),
                                              shift_mode))
              && (new_rtx = simplify_const_binary_operation
@@ -10239,6 +10240,7 @@ simplify_shift_const_1 (enum rtx_code code, enum machine
             for some (ashiftrt (xor)).  */
          if (CONST_INT_P (XEXP (varop, 1))
             && !(code == ASHIFTRT && GET_CODE (varop) == XOR
+                 && result_mode != shift_mode
                  && 0 > trunc_int_for_mode (INTVAL (XEXP (varop, 1)),
                                             shift_mode)))



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]