* [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
@ 2016-05-13 12:21 Kyrill Tkachov
2016-05-13 13:02 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2016-05-13 12:21 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 931 bytes --]
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."
The constant C here is const_op.
Bootstrapped and tested on arm-none-linux-gnueabihf, aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
2016-05-13 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR middle-end/71074
* combine.c (simplify_comparison): Avoid left shift of negative
const_op in LSHIFTRT case.
2016-05-13 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR middle-end/71074
* gcc.c-torture/compile/pr71074.c: New test.
[-- Attachment #2: combine-const_op.patch --]
[-- Type: text/x-patch, Size: 1069 bytes --]
diff --git a/gcc/combine.c b/gcc/combine.c
index 2a7a9e6e2b597246392ede22552af1bdd7e1a794..7a21d593777ef267942e0ee80e024b147907652f 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -12321,6 +12321,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
optimization and for > or <= by setting all the low
order N bits in the comparison constant. */
if (CONST_INT_P (XEXP (op0, 1))
+ && const_op >= 0
&& INTVAL (XEXP (op0, 1)) > 0
&& INTVAL (XEXP (op0, 1)) < HOST_BITS_PER_WIDE_INT
&& mode_width <= HOST_BITS_PER_WIDE_INT
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71074.c b/gcc/testsuite/gcc.c-torture/compile/pr71074.c
new file mode 100644
index 0000000000000000000000000000000000000000..9ad6cbe7c231069c86d3ade22784f338f331b657
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71074.c
@@ -0,0 +1,13 @@
+int bar (void);
+
+void
+foo (unsigned long long a, int b)
+{
+ int i;
+
+ for (a = -12; a >= 10; a = bar ())
+ break;
+
+ if (i == bar () || bar () >= a)
+ bar ();
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
2016-05-13 12:21 [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison Kyrill Tkachov
@ 2016-05-13 13:02 ` Bernd Schmidt
2016-05-13 13:22 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2016-05-13 13:02 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
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
[-- Attachment #2: uconst.diff --]
[-- Type: text/x-patch, Size: 9271 bytes --]
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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
2016-05-13 13:02 ` Bernd Schmidt
@ 2016-05-13 13:22 ` Kyrill Tkachov
2016-05-13 13:26 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2016-05-13 13:22 UTC (permalink / raw)
To: Bernd Schmidt, GCC Patches
On 13/05/16 14:01, Bernd Schmidt wrote:
> 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?
>
Looks reasonable to me barring some comments below.
I'll test a version of the patch with the comments fixed.
Thanks,
Kyrill
>
> 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;
This has to be an rtx rather than HOST_WIDE_INT from the way you use it later on.
/* 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;
Can this be just "unsigned HOST_WIDE_INT uconst_op = UINTVAL (op1);" ?
...
+ 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)
{
(unsigned HOST_WIDE_INT) 1 can be replaced with HOST_WIDE_INT_1U.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
2016-05-13 13:22 ` Kyrill Tkachov
@ 2016-05-13 13:26 ` Bernd Schmidt
2016-06-03 8:36 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2016-05-13 13:26 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches
On 05/13/2016 03:22 PM, Kyrill Tkachov wrote:
> /* 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;
>
> Can this be just "unsigned HOST_WIDE_INT uconst_op = UINTVAL (op1);" ?
Either should work.
> + 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)
> {
>
> (unsigned HOST_WIDE_INT) 1 can be replaced with HOST_WIDE_INT_1U.
Ah, I suspected there was something like this, but none of the
surrounding code was using it. Newly changed code should probably use
that; we could probably improve things further by using it more
consistently in this function, but let's do that in another patch.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
2016-05-13 13:26 ` Bernd Schmidt
@ 2016-06-03 8:36 ` Kyrill Tkachov
2016-06-03 8:57 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2016-06-03 8:36 UTC (permalink / raw)
To: Bernd Schmidt, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]
On 13/05/16 14:26, Bernd Schmidt wrote:
> On 05/13/2016 03:22 PM, Kyrill Tkachov wrote:
>> /* 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;
>>
>> Can this be just "unsigned HOST_WIDE_INT uconst_op = UINTVAL (op1);" ?
>
> Either should work.
>
>> + 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)
>> {
>>
>> (unsigned HOST_WIDE_INT) 1 can be replaced with HOST_WIDE_INT_1U.
>
> Ah, I suspected there was something like this, but none of the surrounding code was using it. Newly changed code should probably use that; we could probably improve things further by using it more consistently in this function, but let's
> do that in another patch.
>
>
> Bernd
Hi Bernd,
Here is the patch with the changes discussed.
Bootstrapped and tested on arm-none-linux-gnueabihf, aarch64-none-linux-gnu and x86_64-linux.
Is this ok?
Thanks,
Kyrill
2016-06-03 Bernd Schmidt <bschmidt@redhat.com>
Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR middle-end/71074
* combine.c (simplify_comparison): Factor out XEXP (op, 1) and
UINTVAL (op1). Avoid left shift of negative value.
2016-06-03 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR middle-end/71074
* gcc.c-torture/compile/pr71074.c: New test.
[-- Attachment #2: combine-ovfw.patch --]
[-- Type: text/x-patch, Size: 10036 bytes --]
diff --git a/gcc/combine.c b/gcc/combine.c
index 0343c3af0ff53199422111c2e40a1afa13ce4e91..752393b7e0e002c0c8152aaa410716f8f3970f82 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11630,13 +11630,13 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
while (CONST_INT_P (op1))
{
+ rtx 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
@@ -11651,7 +11651,8 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
/* 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 = UINTVAL (op1);
/* Compute some predicates to simplify code below. */
@@ -11901,7 +11902,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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))
{
@@ -12206,28 +12207,28 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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)))
- - 1)) == 0)
+ && ((uconst_op
+ & ((HOST_WIDE_INT_1U << 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;
@@ -12235,13 +12236,13 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
/* 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;
}
@@ -12250,8 +12251,8 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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;
@@ -12259,24 +12260,25 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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)
@@ -12290,12 +12292,12 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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)
@@ -12304,7 +12306,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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),
@@ -12314,6 +12316,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
/* ... 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
@@ -12321,21 +12324,20 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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
+ = (HOST_WIDE_INT_1U << 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
@@ -12343,13 +12345,12 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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;
}
@@ -12359,8 +12360,8 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
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);
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71074.c b/gcc/testsuite/gcc.c-torture/compile/pr71074.c
new file mode 100644
index 0000000000000000000000000000000000000000..9ad6cbe7c231069c86d3ade22784f338f331b657
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71074.c
@@ -0,0 +1,13 @@
+int bar (void);
+
+void
+foo (unsigned long long a, int b)
+{
+ int i;
+
+ for (a = -12; a >= 10; a = bar ())
+ break;
+
+ if (i == bar () || bar () >= a)
+ bar ();
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison
2016-06-03 8:36 ` Kyrill Tkachov
@ 2016-06-03 8:57 ` Bernd Schmidt
0 siblings, 0 replies; 6+ messages in thread
From: Bernd Schmidt @ 2016-06-03 8:57 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches
On 06/03/2016 10:35 AM, Kyrill Tkachov wrote:
> Here is the patch with the changes discussed.
> Bootstrapped and tested on arm-none-linux-gnueabihf,
> aarch64-none-linux-gnu and x86_64-linux.
> Is this ok?
I think so, but since this is one of those situations where I'm
essentially approving my own patch, let's wait a few days in case anyone
has objections. Ok to commit afterwards.
> PR middle-end/71074
> * gcc.c-torture/compile/pr71074.c: New test.
Not strictly required in the testsuite I think, but it would be nice if
the testcase was formatted better.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-03 8:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 12:21 [PATCH][combine] PR middle-end/71074 Check that const_op is >= 0 before potentially shifting in simplify_comparison Kyrill Tkachov
2016-05-13 13:02 ` Bernd Schmidt
2016-05-13 13:22 ` Kyrill Tkachov
2016-05-13 13:26 ` Bernd Schmidt
2016-06-03 8:36 ` Kyrill Tkachov
2016-06-03 8:57 ` Bernd Schmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).