| Message ID | d61d30d2-9996-c886-2502-519678453d2c@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Hi Bill, Is there a reason the new test is in gcc.dg rather than in gcc.dg/ubsan? The test FAILs when there is no ubsan runtime support and fsanitize_undefined effective target is not available in gcc.dg (one needs to load ubsan-dg for this effective target to be defined). Best regards, Thomas On 14/07/17 19:05, Bill Schmidt wrote: > Hi, > > PR81162 identifies a bug in SLSR involving overflow that occurs when > replacing a NEGATE_EXPR with a PLUS_EXPR. This is another example > of an unprofitable transformation that should be skipped anyway, > hence this simple patch. Bootstrapped and tested on > powerpc64le-unknown-linux-gnu, committed. Test case provided from > the bug report. > > Thanks, > Bill > > > [gcc] > > 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/81162 > * gimple-ssa-strength-reduction.c (replace_mult_candidate): Don't > replace a negate with an add. > > [gcc/testsuite] > > 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/81162 > * gcc.dg/pr81162.c: New file. > > > Index: gcc/gimple-ssa-strength-reduction.c > =================================================================== > --- gcc/gimple-ssa-strength-reduction.c (revision 250189) > +++ gcc/gimple-ssa-strength-reduction.c (working copy) > @@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ > types but allows for safe negation without twisted logic. */ > if (wi::fits_shwi_p (bump) > && bump.to_shwi () != HOST_WIDE_INT_MIN > - /* It is not useful to replace casts, copies, or adds of > + /* It is not useful to replace casts, copies, negates, or adds of > an SSA name and a constant. */ > && cand_code != SSA_NAME > && !CONVERT_EXPR_CODE_P (cand_code) > && cand_code != PLUS_EXPR > && cand_code != POINTER_PLUS_EXPR > - && cand_code != MINUS_EXPR) > + && cand_code != MINUS_EXPR > + && cand_code != NEGATE_EXPR) > { > enum tree_code code = PLUS_EXPR; > tree bump_tree; > Index: gcc/testsuite/gcc.dg/pr81162.c > =================================================================== > --- gcc/testsuite/gcc.dg/pr81162.c (nonexistent) > +++ gcc/testsuite/gcc.dg/pr81162.c (working copy) > @@ -0,0 +1,17 @@ > +/* PR tree-optimization/81162 */ > +/* { dg-do run } */ > +/* { dg-options "-fsanitize=undefined -O2" } */ > + > +short s; > +int i1 = 1; > +int i2 = 1; > +unsigned char uc = 147; > + > +int main() { > + s = (-uc + 2147483647) << 0; > + if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) { > + return 0; > + } else { > + return -1; > + } > +} >
Thomas, thanks for the heads-up! I didn't realize we had this dependency. I'll move the test case shortly. -- Bill > On Jul 17, 2017, at 5:47 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > Hi Bill, > > Is there a reason the new test is in gcc.dg rather than in gcc.dg/ubsan? The test FAILs when there is no ubsan runtime support and fsanitize_undefined effective target is not available in gcc.dg (one needs to load ubsan-dg for this effective target to be defined). > > Best regards, > > Thomas > > On 14/07/17 19:05, Bill Schmidt wrote: >> Hi, >> PR81162 identifies a bug in SLSR involving overflow that occurs when >> replacing a NEGATE_EXPR with a PLUS_EXPR. This is another example >> of an unprofitable transformation that should be skipped anyway, >> hence this simple patch. Bootstrapped and tested on >> powerpc64le-unknown-linux-gnu, committed. Test case provided from >> the bug report. >> Thanks, >> Bill >> [gcc] >> 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> >> PR tree-optimization/81162 >> * gimple-ssa-strength-reduction.c (replace_mult_candidate): Don't >> replace a negate with an add. >> [gcc/testsuite] >> 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> >> PR tree-optimization/81162 >> * gcc.dg/pr81162.c: New file. >> Index: gcc/gimple-ssa-strength-reduction.c >> =================================================================== >> --- gcc/gimple-ssa-strength-reduction.c (revision 250189) >> +++ gcc/gimple-ssa-strength-reduction.c (working copy) >> @@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ >> types but allows for safe negation without twisted logic. */ >> if (wi::fits_shwi_p (bump) >> && bump.to_shwi () != HOST_WIDE_INT_MIN >> - /* It is not useful to replace casts, copies, or adds of >> + /* It is not useful to replace casts, copies, negates, or adds of >> an SSA name and a constant. */ >> && cand_code != SSA_NAME >> && !CONVERT_EXPR_CODE_P (cand_code) >> && cand_code != PLUS_EXPR >> && cand_code != POINTER_PLUS_EXPR >> - && cand_code != MINUS_EXPR) >> + && cand_code != MINUS_EXPR >> + && cand_code != NEGATE_EXPR) >> { >> enum tree_code code = PLUS_EXPR; >> tree bump_tree; >> Index: gcc/testsuite/gcc.dg/pr81162.c >> =================================================================== >> --- gcc/testsuite/gcc.dg/pr81162.c (nonexistent) >> +++ gcc/testsuite/gcc.dg/pr81162.c (working copy) >> @@ -0,0 +1,17 @@ >> +/* PR tree-optimization/81162 */ >> +/* { dg-do run } */ >> +/* { dg-options "-fsanitize=undefined -O2" } */ >> + >> +short s; >> +int i1 = 1; >> +int i2 = 1; >> +unsigned char uc = 147; >> + >> +int main() { >> + s = (-uc + 2147483647) << 0; >> + if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) { >> + return 0; >> + } else { >> + return -1; >> + } >> +} >
Hi Richard, I would like to backport this fix to GCC 5, 6, and 7. All have passed regstrap on powerpc64le-linux-gnu (with the test case moved to gcc.dg/ubsan, of course). Is this ok? Thanks! -- Bill > On Jul 14, 2017, at 1:05 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > > Hi, > > PR81162 identifies a bug in SLSR involving overflow that occurs when > replacing a NEGATE_EXPR with a PLUS_EXPR. This is another example > of an unprofitable transformation that should be skipped anyway, > hence this simple patch. Bootstrapped and tested on > powerpc64le-unknown-linux-gnu, committed. Test case provided from > the bug report. > > Thanks, > Bill > > > [gcc] > > 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/81162 > * gimple-ssa-strength-reduction.c (replace_mult_candidate): Don't > replace a negate with an add. > > [gcc/testsuite] > > 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/81162 > * gcc.dg/pr81162.c: New file. > > > Index: gcc/gimple-ssa-strength-reduction.c > =================================================================== > --- gcc/gimple-ssa-strength-reduction.c (revision 250189) > +++ gcc/gimple-ssa-strength-reduction.c (working copy) > @@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ > types but allows for safe negation without twisted logic. */ > if (wi::fits_shwi_p (bump) > && bump.to_shwi () != HOST_WIDE_INT_MIN > - /* It is not useful to replace casts, copies, or adds of > + /* It is not useful to replace casts, copies, negates, or adds of > an SSA name and a constant. */ > && cand_code != SSA_NAME > && !CONVERT_EXPR_CODE_P (cand_code) > && cand_code != PLUS_EXPR > && cand_code != POINTER_PLUS_EXPR > - && cand_code != MINUS_EXPR) > + && cand_code != MINUS_EXPR > + && cand_code != NEGATE_EXPR) > { > enum tree_code code = PLUS_EXPR; > tree bump_tree; > Index: gcc/testsuite/gcc.dg/pr81162.c > =================================================================== > --- gcc/testsuite/gcc.dg/pr81162.c (nonexistent) > +++ gcc/testsuite/gcc.dg/pr81162.c (working copy) > @@ -0,0 +1,17 @@ > +/* PR tree-optimization/81162 */ > +/* { dg-do run } */ > +/* { dg-options "-fsanitize=undefined -O2" } */ > + > +short s; > +int i1 = 1; > +int i2 = 1; > +unsigned char uc = 147; > + > +int main() { > + s = (-uc + 2147483647) << 0; > + if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) { > + return 0; > + } else { > + return -1; > + } > +} >
On Fri, Jul 21, 2017 at 7:40 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > Hi Richard, > > I would like to backport this fix to GCC 5, 6, and 7. All have passed regstrap on > powerpc64le-linux-gnu (with the test case moved to gcc.dg/ubsan, of course). > Is this ok? Yes. Richard. > Thanks! > > -- Bill > > >> On Jul 14, 2017, at 1:05 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: >> >> Hi, >> >> PR81162 identifies a bug in SLSR involving overflow that occurs when >> replacing a NEGATE_EXPR with a PLUS_EXPR. This is another example >> of an unprofitable transformation that should be skipped anyway, >> hence this simple patch. Bootstrapped and tested on >> powerpc64le-unknown-linux-gnu, committed. Test case provided from >> the bug report. >> >> Thanks, >> Bill >> >> >> [gcc] >> >> 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> >> >> PR tree-optimization/81162 >> * gimple-ssa-strength-reduction.c (replace_mult_candidate): Don't >> replace a negate with an add. >> >> [gcc/testsuite] >> >> 2016-07-14 Bill Schmidt <wschmidt@linux.vnet.ibm.com> >> >> PR tree-optimization/81162 >> * gcc.dg/pr81162.c: New file. >> >> >> Index: gcc/gimple-ssa-strength-reduction.c >> =================================================================== >> --- gcc/gimple-ssa-strength-reduction.c (revision 250189) >> +++ gcc/gimple-ssa-strength-reduction.c (working copy) >> @@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ >> types but allows for safe negation without twisted logic. */ >> if (wi::fits_shwi_p (bump) >> && bump.to_shwi () != HOST_WIDE_INT_MIN >> - /* It is not useful to replace casts, copies, or adds of >> + /* It is not useful to replace casts, copies, negates, or adds of >> an SSA name and a constant. */ >> && cand_code != SSA_NAME >> && !CONVERT_EXPR_CODE_P (cand_code) >> && cand_code != PLUS_EXPR >> && cand_code != POINTER_PLUS_EXPR >> - && cand_code != MINUS_EXPR) >> + && cand_code != MINUS_EXPR >> + && cand_code != NEGATE_EXPR) >> { >> enum tree_code code = PLUS_EXPR; >> tree bump_tree; >> Index: gcc/testsuite/gcc.dg/pr81162.c >> =================================================================== >> --- gcc/testsuite/gcc.dg/pr81162.c (nonexistent) >> +++ gcc/testsuite/gcc.dg/pr81162.c (working copy) >> @@ -0,0 +1,17 @@ >> +/* PR tree-optimization/81162 */ >> +/* { dg-do run } */ >> +/* { dg-options "-fsanitize=undefined -O2" } */ >> + >> +short s; >> +int i1 = 1; >> +int i2 = 1; >> +unsigned char uc = 147; >> + >> +int main() { >> + s = (-uc + 2147483647) << 0; >> + if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) { >> + return 0; >> + } else { >> + return -1; >> + } >> +} >> >
Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 250189) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ types but allows for safe negation without twisted logic. */ if (wi::fits_shwi_p (bump) && bump.to_shwi () != HOST_WIDE_INT_MIN - /* It is not useful to replace casts, copies, or adds of + /* It is not useful to replace casts, copies, negates, or adds of an SSA name and a constant. */ && cand_code != SSA_NAME && !CONVERT_EXPR_CODE_P (cand_code) && cand_code != PLUS_EXPR && cand_code != POINTER_PLUS_EXPR - && cand_code != MINUS_EXPR) + && cand_code != MINUS_EXPR + && cand_code != NEGATE_EXPR) { enum tree_code code = PLUS_EXPR; tree bump_tree; Index: gcc/testsuite/gcc.dg/pr81162.c =================================================================== --- gcc/testsuite/gcc.dg/pr81162.c (nonexistent) +++ gcc/testsuite/gcc.dg/pr81162.c (working copy) @@ -0,0 +1,17 @@ +/* PR tree-optimization/81162 */ +/* { dg-do run } */ +/* { dg-options "-fsanitize=undefined -O2" } */ + +short s; +int i1 = 1; +int i2 = 1; +unsigned char uc = 147; + +int main() { + s = (-uc + 2147483647) << 0; + if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) { + return 0; + } else { + return -1; + } +}