| Message ID | 559D3BD3.1010808@arm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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 >
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; +}
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.