* [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function.
@ 2015-07-08 15:03 Renlin Li
2015-07-08 16:41 ` Segher Boessenkool
2015-07-08 20:56 ` Jeff Law
0 siblings, 2 replies; 4+ messages in thread
From: Renlin Li @ 2015-07-08 15:03 UTC (permalink / raw)
To: gcc-patches; +Cc: segher, Ramana Radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
Hi all,
In simplify_const_relational_operation function, there are cases a const rtx
will be returned.
Three cases are considered in this function:
1, comparisons with upper and lower bounds.
2, Integer comparisons with zero.
3, comparison of ABS with zero.
It's fine to to the optimization if the operands have no side-effects.
For example, I am currently fixing a code generation bug for armv7-a
bigendian.
It turns out that, the following rtx is simplified into a const, and the
side-effect with it is ignored.
(ltu:SI (lshiftrt:SI (subreg:SI (mem/c:HI (post_modify:SI (reg/f:SI 156)
(plus:SI (reg/f:SI 156)
(const_int 20 [0x14]))) [5 g+4 S2 A32]) 0)
(const_int 1 [0x1]))
(const_int -1 [0xffffffffffffffff]))
------------>>>>>>>>>>>>>>>>>>
(const_int 1 [0x1])
This particular case falls into category 1 mentioned above. -1, when
regarded
as unsigned integer, is the largest unsigned integer. So the result is
always
a const_true_rtx in this case. However, the first operand of the comparison
has POST_MODIFY side-effect.
In this case, the simplifications should be checked against side-effect.
x86_64 bootstrapping is Okay and arm-none-eabi regression test runs
without any new issues.
Okay to commit and backport to branch 5?
Regards,
Renlin Li
gcc/ChangeLog:
2015-07-08 Renlin Li <renlin.li@arm.com>
PR rtl/66556
* simplify-rtx.c (simplify_const_relational_operation): Add
side_effects_p check.
gcc/testsuite/ChangeLog:
2015-07-08 Renlin Li <renlin.li@arm.com>
PR rtl/66556
* gcc.c-torture/execute/pr66556.c: New.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: new.diff --]
[-- Type: text/x-patch; name=new.diff, Size: 2187 bytes --]
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index ca8310d..936a144 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -4930,7 +4930,8 @@ simplify_const_relational_operation (enum rtx_code code,
/* Optimize comparisons with upper and lower bounds. */
if (HWI_COMPUTABLE_MODE_P (mode)
- && CONST_INT_P (trueop1))
+ && CONST_INT_P (trueop1)
+ && !side_effects_p (trueop0))
{
int sign;
unsigned HOST_WIDE_INT nonzero = nonzero_bits (trueop0, mode);
@@ -5043,7 +5044,7 @@ simplify_const_relational_operation (enum rtx_code code,
}
/* Optimize integer comparisons with zero. */
- if (trueop1 == const0_rtx)
+ if (trueop1 == const0_rtx && !side_effects_p (trueop0))
{
/* Some addresses are known to be nonzero. We don't know
their sign, but equality comparisons are known. */
@@ -5094,7 +5095,7 @@ simplify_const_relational_operation (enum rtx_code code,
}
/* Optimize comparison of ABS with zero. */
- if (trueop1 == CONST0_RTX (mode)
+ if (trueop1 == CONST0_RTX (mode) && !side_effects_p (trueop0)
&& (GET_CODE (trueop0) == ABS
|| (GET_CODE (trueop0) == FLOAT_EXTEND
&& GET_CODE (XEXP (trueop0, 0)) == ABS)))
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr66556.c b/gcc/testsuite/gcc.c-torture/execute/pr66556.c
new file mode 100644
index 0000000..f7acf1c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr66556.c
@@ -0,0 +1,52 @@
+/* { dg-do run } */
+
+extern void abort (void);
+
+struct {
+ unsigned f2;
+ unsigned f3 : 15;
+ unsigned f5 : 3;
+ short f6;
+} b = {0x7f8000, 6, 5, 0}, g = {8, 0, 5, 0};
+
+short d, l;
+int a, c, h = 8;
+volatile char e[237] = {4};
+short *f = &d;
+short i[5] = {3};
+char j;
+int *k = &c;
+
+int
+fn1 (unsigned p1) { return -p1; }
+
+void
+fn2 (char p1)
+{
+ a = p1;
+ e[0];
+}
+
+short
+fn3 ()
+{
+ *k = 4;
+ return *f;
+}
+
+int
+main ()
+{
+
+ unsigned m;
+ short *n = &i[4];
+
+ m = fn1 ((h && j) <= b.f5);
+ l = m > g.f3;
+ *n = 3;
+ fn2 (b.f2 >> 15);
+ if ((a & 0xff) != 0xff)
+ abort ();
+
+ return 0;
+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function.
2015-07-08 15:03 [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function Renlin Li
@ 2015-07-08 16:41 ` Segher Boessenkool
2015-07-08 20:56 ` Jeff Law
1 sibling, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2015-07-08 16:41 UTC (permalink / raw)
To: Renlin Li; +Cc: gcc-patches, segher, Ramana Radhakrishnan
On Wed, Jul 08, 2015 at 04:03:47PM +0100, Renlin Li wrote:
> PR rtl/66556
> * simplify-rtx.c (simplify_const_relational_operation): Add
> side_effects_p check.
"checks"?
The patch looks good to me, but someone else will need to approve.
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function.
2015-07-08 15:03 [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function Renlin Li
2015-07-08 16:41 ` Segher Boessenkool
@ 2015-07-08 20:56 ` Jeff Law
2015-07-10 9:57 ` Renlin Li
1 sibling, 1 reply; 4+ messages in thread
From: Jeff Law @ 2015-07-08 20:56 UTC (permalink / raw)
To: Renlin Li, gcc-patches; +Cc: segher, Ramana Radhakrishnan
On 07/08/2015 09:03 AM, Renlin Li wrote:
> Hi all,
>
> In simplify_const_relational_operation function, there are cases a const
> rtx
> will be returned.
>
> Three cases are considered in this function:
> 1, comparisons with upper and lower bounds.
> 2, Integer comparisons with zero.
> 3, comparison of ABS with zero.
>
> It's fine to to the optimization if the operands have no side-effects.
>
> For example, I am currently fixing a code generation bug for armv7-a
> bigendian.
> It turns out that, the following rtx is simplified into a const, and the
> side-effect with it is ignored.
>
> (ltu:SI (lshiftrt:SI (subreg:SI (mem/c:HI (post_modify:SI (reg/f:SI 156)
> (plus:SI (reg/f:SI 156)
> (const_int 20 [0x14]))) [5 g+4 S2 A32]) 0)
> (const_int 1 [0x1]))
> (const_int -1 [0xffffffffffffffff]))
>
> ------------>>>>>>>>>>>>>>>>>>
>
> (const_int 1 [0x1])
>
> This particular case falls into category 1 mentioned above. -1, when
> regarded
> as unsigned integer, is the largest unsigned integer. So the result is
> always
> a const_true_rtx in this case. However, the first operand of the comparison
> has POST_MODIFY side-effect.
>
> In this case, the simplifications should be checked against side-effect.
>
> x86_64 bootstrapping is Okay and arm-none-eabi regression test runs
> without any new issues.
>
> Okay to commit and backport to branch 5?
>
> Regards,
> Renlin Li
>
> gcc/ChangeLog:
>
> 2015-07-08 Renlin Li <renlin.li@arm.com>
>
> PR rtl/66556
> * simplify-rtx.c (simplify_const_relational_operation): Add
> side_effects_p check.
>
> gcc/testsuite/ChangeLog:
>
> 2015-07-08 Renlin Li <renlin.li@arm.com>
>
> PR rtl/66556
> * gcc.c-torture/execute/pr66556.c: New.
OK.
It may be worth looking at the .optimized dump for the new test and see
if there's something we can/should be optimizing better before we start
generating RTL. That can obviously be a follow-up.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function.
2015-07-08 20:56 ` Jeff Law
@ 2015-07-10 9:57 ` Renlin Li
0 siblings, 0 replies; 4+ messages in thread
From: Renlin Li @ 2015-07-10 9:57 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: segher, Ramana Radhakrishnan
Hi Jeff,
Thank you for the suggestion! I will committed it first and continue
working on it.
Regards,
Renlin Li
On 08/07/15 21:56, Jeff Law wrote:
> On 07/08/2015 09:03 AM, Renlin Li wrote:
>> Hi all,
>>
>> In simplify_const_relational_operation function, there are cases a const
>> rtx
>> will be returned.
>>
>> Three cases are considered in this function:
>> 1, comparisons with upper and lower bounds.
>> 2, Integer comparisons with zero.
>> 3, comparison of ABS with zero.
>>
>> It's fine to to the optimization if the operands have no side-effects.
>>
>> For example, I am currently fixing a code generation bug for armv7-a
>> bigendian.
>> It turns out that, the following rtx is simplified into a const, and the
>> side-effect with it is ignored.
>>
>> (ltu:SI (lshiftrt:SI (subreg:SI (mem/c:HI (post_modify:SI (reg/f:SI 156)
>> (plus:SI (reg/f:SI 156)
>> (const_int 20 [0x14]))) [5 g+4 S2 A32]) 0)
>> (const_int 1 [0x1]))
>> (const_int -1 [0xffffffffffffffff]))
>>
>> ------------>>>>>>>>>>>>>>>>>>
>>
>> (const_int 1 [0x1])
>>
>> This particular case falls into category 1 mentioned above. -1, when
>> regarded
>> as unsigned integer, is the largest unsigned integer. So the result is
>> always
>> a const_true_rtx in this case. However, the first operand of the comparison
>> has POST_MODIFY side-effect.
>>
>> In this case, the simplifications should be checked against side-effect.
>>
>> x86_64 bootstrapping is Okay and arm-none-eabi regression test runs
>> without any new issues.
>>
>> Okay to commit and backport to branch 5?
>>
>> Regards,
>> Renlin Li
>>
>> gcc/ChangeLog:
>>
>> 2015-07-08 Renlin Li<renlin.li@arm.com>
>>
>> PR rtl/66556
>> * simplify-rtx.c (simplify_const_relational_operation): Add
>> side_effects_p check.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-07-08 Renlin Li<renlin.li@arm.com>
>>
>> PR rtl/66556
>> * gcc.c-torture/execute/pr66556.c: New.
> OK.
>
> It may be worth looking at the .optimized dump for the new test and see
> if there's something we can/should be optimizing better before we start
> generating RTL. That can obviously be a follow-up.
>
> jeff
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-10 9:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-08 15:03 [PATCH]Fix PR66556. Don't drop side-effect in simplify_const_relational_operation function Renlin Li
2015-07-08 16:41 ` Segher Boessenkool
2015-07-08 20:56 ` Jeff Law
2015-07-10 9:57 ` Renlin Li
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).