This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 13 May 2016 15:01:55 +0200
- Subject: Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
- Authentication-results: sourceware.org; auth=none
- References: <5735C6AF dot 8040002 at foss dot arm dot com>
On 05/13/2016 02:21 PM, Kyrill Tkachov wrote:
Hi all,
In this PR we may end up shifting a negative value left in
simplify_comparison.
The statement is:
const_op <<= INTVAL (XEXP (op0, 1));
This patch guards the block of that statement by const_op >= 0.
I _think_ that's a correct thing to do for that trasnformation:
"If we have (compare (xshiftrt FOO N) (const_int C)) and
the low order N bits of FOO are known to be zero, we can do this
by comparing FOO with C shifted left N bits so long as no
overflow occurs."
Isn't the condition testing for overflow though? In a somewhat
nonobvious way.
So I think you should just use an unsigned version of const_op. While
we're here we might as well make the code here a little more readable.
How about the patch below? Can you test whether this works for you?
Bernd
Index: combine.c
===================================================================
--- combine.c (revision 236113)
+++ combine.c (working copy)
@@ -11628,13 +11628,13 @@ simplify_comparison (enum rtx_code code,
while (CONST_INT_P (op1))
{
+ HOST_WIDE_INT amount;
machine_mode mode = GET_MODE (op0);
unsigned int mode_width = GET_MODE_PRECISION (mode);
unsigned HOST_WIDE_INT mask = GET_MODE_MASK (mode);
int equality_comparison_p;
int sign_bit_comparison_p;
int unsigned_comparison_p;
- HOST_WIDE_INT const_op;
/* We only want to handle integral modes. This catches VOIDmode,
CCmode, and the floating-point modes. An exception is that we
@@ -11649,7 +11649,8 @@ simplify_comparison (enum rtx_code code,
/* Try to simplify the compare to constant, possibly changing the
comparison op, and/or changing op1 to zero. */
code = simplify_compare_const (code, mode, op0, &op1);
- const_op = INTVAL (op1);
+ HOST_WIDE_INT const_op = INTVAL (op1);
+ unsigned HOST_WIDE_INT uconst_op = (unsigned HOST_WIDE_INT) const_op;
/* Compute some predicates to simplify code below. */
@@ -11899,7 +11900,7 @@ simplify_comparison (enum rtx_code code,
if (GET_MODE_CLASS (mode) == MODE_INT
&& (unsigned_comparison_p || equality_comparison_p)
&& HWI_COMPUTABLE_MODE_P (mode)
- && (unsigned HOST_WIDE_INT) const_op <= GET_MODE_MASK (mode)
+ && uconst_op <= GET_MODE_MASK (mode)
&& const_op >= 0
&& have_insn_for (COMPARE, mode))
{
@@ -12198,28 +12199,29 @@ simplify_comparison (enum rtx_code code,
break;
case ASHIFT:
+ amount = XEXP (op0, 1);
/* If we have (compare (ashift FOO N) (const_int C)) and
the high order N bits of FOO (N+1 if an inequality comparison)
are known to be zero, we can do this by comparing FOO with C
shifted right N bits so long as the low-order N bits of C are
zero. */
- if (CONST_INT_P (XEXP (op0, 1))
- && INTVAL (XEXP (op0, 1)) >= 0
- && ((INTVAL (XEXP (op0, 1)) + ! equality_comparison_p)
+ if (CONST_INT_P (amount)
+ && INTVAL (amount) >= 0
+ && ((INTVAL (amount) + ! equality_comparison_p)
< HOST_BITS_PER_WIDE_INT)
- && (((unsigned HOST_WIDE_INT) const_op
- & (((unsigned HOST_WIDE_INT) 1 << INTVAL (XEXP (op0, 1)))
+ && ((uconst_op
+ & (((unsigned HOST_WIDE_INT) 1 << INTVAL (amount))
- 1)) == 0)
&& mode_width <= HOST_BITS_PER_WIDE_INT
&& (nonzero_bits (XEXP (op0, 0), mode)
- & ~(mask >> (INTVAL (XEXP (op0, 1))
+ & ~(mask >> (INTVAL (amount)
+ ! equality_comparison_p))) == 0)
{
/* We must perform a logical shift, not an arithmetic one,
as we want the top N bits of C to be zero. */
unsigned HOST_WIDE_INT temp = const_op & GET_MODE_MASK (mode);
- temp >>= INTVAL (XEXP (op0, 1));
+ temp >>= INTVAL (amount);
op1 = gen_int_mode (temp, mode);
op0 = XEXP (op0, 0);
continue;
@@ -12227,13 +12229,13 @@ simplify_comparison (enum rtx_code code,
/* If we are doing a sign bit comparison, it means we are testing
a particular bit. Convert it to the appropriate AND. */
- if (sign_bit_comparison_p && CONST_INT_P (XEXP (op0, 1))
+ if (sign_bit_comparison_p && CONST_INT_P (amount)
&& mode_width <= HOST_BITS_PER_WIDE_INT)
{
op0 = simplify_and_const_int (NULL_RTX, mode, XEXP (op0, 0),
((unsigned HOST_WIDE_INT) 1
<< (mode_width - 1
- - INTVAL (XEXP (op0, 1)))));
+ - INTVAL (amount))));
code = (code == LT ? NE : EQ);
continue;
}
@@ -12242,8 +12244,8 @@ simplify_comparison (enum rtx_code code,
the low bit to the sign bit, we can convert this to an AND of the
low-order bit. */
if (const_op == 0 && equality_comparison_p
- && CONST_INT_P (XEXP (op0, 1))
- && UINTVAL (XEXP (op0, 1)) == mode_width - 1)
+ && CONST_INT_P (amount)
+ && UINTVAL (amount) == mode_width - 1)
{
op0 = simplify_and_const_int (NULL_RTX, mode, XEXP (op0, 0), 1);
continue;
@@ -12251,24 +12253,25 @@ simplify_comparison (enum rtx_code code,
break;
case ASHIFTRT:
+ amount = XEXP (op0, 1);
/* If this is an equality comparison with zero, we can do this
as a logical shift, which might be much simpler. */
if (equality_comparison_p && const_op == 0
- && CONST_INT_P (XEXP (op0, 1)))
+ && CONST_INT_P (amount))
{
op0 = simplify_shift_const (NULL_RTX, LSHIFTRT, mode,
XEXP (op0, 0),
- INTVAL (XEXP (op0, 1)));
+ INTVAL (amount));
continue;
}
/* If OP0 is a sign extension and CODE is not an unsigned comparison,
do the comparison in a narrower mode. */
if (! unsigned_comparison_p
- && CONST_INT_P (XEXP (op0, 1))
+ && CONST_INT_P (amount)
&& GET_CODE (XEXP (op0, 0)) == ASHIFT
- && XEXP (op0, 1) == XEXP (XEXP (op0, 0), 1)
- && (tmode = mode_for_size (mode_width - INTVAL (XEXP (op0, 1)),
+ && amount == XEXP (XEXP (op0, 0), 1)
+ && (tmode = mode_for_size (mode_width - INTVAL (amount),
MODE_INT, 1)) != BLKmode
&& (((unsigned HOST_WIDE_INT) const_op
+ (GET_MODE_MASK (tmode) >> 1) + 1)
@@ -12282,12 +12285,12 @@ simplify_comparison (enum rtx_code code,
constant, which is usually represented with the PLUS
between the shifts. */
if (! unsigned_comparison_p
- && CONST_INT_P (XEXP (op0, 1))
+ && CONST_INT_P (amount)
&& GET_CODE (XEXP (op0, 0)) == PLUS
&& CONST_INT_P (XEXP (XEXP (op0, 0), 1))
&& GET_CODE (XEXP (XEXP (op0, 0), 0)) == ASHIFT
- && XEXP (op0, 1) == XEXP (XEXP (XEXP (op0, 0), 0), 1)
- && (tmode = mode_for_size (mode_width - INTVAL (XEXP (op0, 1)),
+ && amount == XEXP (XEXP (XEXP (op0, 0), 0), 1)
+ && (tmode = mode_for_size (mode_width - INTVAL (amount),
MODE_INT, 1)) != BLKmode
&& (((unsigned HOST_WIDE_INT) const_op
+ (GET_MODE_MASK (tmode) >> 1) + 1)
@@ -12296,7 +12299,7 @@ simplify_comparison (enum rtx_code code,
rtx inner = XEXP (XEXP (XEXP (op0, 0), 0), 0);
rtx add_const = XEXP (XEXP (op0, 0), 1);
rtx new_const = simplify_gen_binary (ASHIFTRT, GET_MODE (op0),
- add_const, XEXP (op0, 1));
+ add_const, amount);
op0 = simplify_gen_binary (PLUS, tmode,
gen_lowpart (tmode, inner),
@@ -12306,6 +12309,7 @@ simplify_comparison (enum rtx_code code,
/* ... fall through ... */
case LSHIFTRT:
+ amount = XEXP (op0, 1);
/* If we have (compare (xshiftrt FOO N) (const_int C)) and
the low order N bits of FOO are known to be zero, we can do this
by comparing FOO with C shifted left N bits so long as no
@@ -12313,21 +12317,20 @@ simplify_comparison (enum rtx_code code,
to be zero, if the comparison is >= or < we can use the same
optimization and for > or <= by setting all the low
order N bits in the comparison constant. */
- if (CONST_INT_P (XEXP (op0, 1))
- && INTVAL (XEXP (op0, 1)) > 0
- && INTVAL (XEXP (op0, 1)) < HOST_BITS_PER_WIDE_INT
+ if (CONST_INT_P (amount)
+ && INTVAL (amount) > 0
+ && INTVAL (amount) < HOST_BITS_PER_WIDE_INT
&& mode_width <= HOST_BITS_PER_WIDE_INT
- && (((unsigned HOST_WIDE_INT) const_op
+ && ((uconst_op
+ (GET_CODE (op0) != LSHIFTRT
- ? ((GET_MODE_MASK (mode) >> INTVAL (XEXP (op0, 1)) >> 1)
- + 1)
+ ? ((GET_MODE_MASK (mode) >> INTVAL (amount) >> 1) + 1)
: 0))
- <= GET_MODE_MASK (mode) >> INTVAL (XEXP (op0, 1))))
+ <= GET_MODE_MASK (mode) >> INTVAL (amount)))
{
+ unsigned HOST_WIDE_INT low_mask
+ = (((unsigned HOST_WIDE_INT) 1 << INTVAL (amount)) - 1);
unsigned HOST_WIDE_INT low_bits
- = (nonzero_bits (XEXP (op0, 0), mode)
- & (((unsigned HOST_WIDE_INT) 1
- << INTVAL (XEXP (op0, 1))) - 1));
+ = (nonzero_bits (XEXP (op0, 0), mode) & low_mask);
if (low_bits == 0 || !equality_comparison_p)
{
/* If the shift was logical, then we must make the condition
@@ -12335,13 +12338,12 @@ simplify_comparison (enum rtx_code code,
if (GET_CODE (op0) == LSHIFTRT)
code = unsigned_condition (code);
- const_op <<= INTVAL (XEXP (op0, 1));
+ uconst_op <<= INTVAL (amount);
if (low_bits != 0
&& (code == GT || code == GTU
|| code == LE || code == LEU))
- const_op
- |= (((HOST_WIDE_INT) 1 << INTVAL (XEXP (op0, 1))) - 1);
- op1 = GEN_INT (const_op);
+ uconst_op |= low_mask;
+ op1 = gen_int_mode (uconst_op, mode);
op0 = XEXP (op0, 0);
continue;
}
@@ -12351,8 +12353,8 @@ simplify_comparison (enum rtx_code code,
can replace this with an LT or GE comparison. */
if (const_op == 0
&& (equality_comparison_p || sign_bit_comparison_p)
- && CONST_INT_P (XEXP (op0, 1))
- && UINTVAL (XEXP (op0, 1)) == mode_width - 1)
+ && CONST_INT_P (amount)
+ && UINTVAL (amount) == mode_width - 1)
{
op0 = XEXP (op0, 0);
code = (code == NE || code == GT ? LT : GE);