| Message ID | 1398356446.659.123.camel@gnopaine |
|---|---|
| State | New |
| Headers | show |
On 04/24/14 10:20, Bill Schmidt wrote: > Hi, > > PR60930 exposes an SLSR problem with a fold. When multiplying two > constants to create a new stride, the result must fit in the stride type > for the computation or the fold is invalid. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no > regressions. The same patch applies equally to 4.8, 4.9, and trunk. Is > this ok for trunk (and for 4.8/4.9 after a suitable burn-in period)? > > Thanks, > Bill > > > [gcc] > > 2014-04-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/60930 > * gimple-ssa-strength-reduction.c (create_mul_imm_cand): Reject > creating a multiply candidate by folding two constant > multiplicands when the result overflows. > > [gcc/testsuite] > > 2014-04-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/60930 > * gcc.dg/torture/pr60930.c: New test. Doesn't the test depend on long long being at least 64 bits? What we've done for these kinds of tests in the past is: if (sizeof (whatever) < needed size) exit (0); Another approach would be to use an effective target test and skip the test if the target doesn't have a suitable long long. Look in testsuite/lib/target-supports.exp for the various target characteristics you can test for. If you go this route you'd create pr60930.x which skips the test. There's several examples you can use to guide you. With the testcase fixed, this is OK. jeff
On Thu, Apr 24, 2014 at 09:20:50PM -0600, Jeff Law wrote: > > PR tree-optimization/60930 > > * gcc.dg/torture/pr60930.c: New test. > Doesn't the test depend on long long being at least 64 bits? But that is guaranteed by C99, isn't it? 5.2.4.2.1 says: ... Their implementation-defined values shall be equal or greater in magnitude (absolute value) to those shown, with the same sign. ... - maximum value for an object of type unsigned long long int ULLONG_MAX 18446744073709551615 // 2 64 − 1 > > What we've done for these kinds of tests in the past is: > > if (sizeof (whatever) < needed size) > exit (0); > > Another approach would be to use an effective target test and skip > the test if the target doesn't have a suitable long long. Look in > testsuite/lib/target-supports.exp for the various target If it was some other type, sure, one could use int32plus, lp64, etc. target, or #if __SIZEOF_type__ == ... Jakub
On Fri, 25 Apr 2014, Jakub Jelinek wrote: > On Thu, Apr 24, 2014 at 09:20:50PM -0600, Jeff Law wrote: > > > PR tree-optimization/60930 > > > * gcc.dg/torture/pr60930.c: New test. > > Doesn't the test depend on long long being at least 64 bits? > > But that is guaranteed by C99, isn't it? But the testcase isn't built with -std=c99. > 5.2.4.2.1 says: > > ... Their implementation-defined values shall be equal or greater in magnitude > (absolute value) to those shown, with the same sign. > ... > - maximum value for an object of type unsigned long long int ULLONG_MAX > 18446744073709551615 // 2 64 − 1 > > > > What we've done for these kinds of tests in the past is: > > > > if (sizeof (whatever) < needed size) > > exit (0); > > > > Another approach would be to use an effective target test and skip > > the test if the target doesn't have a suitable long long. Look in > > testsuite/lib/target-supports.exp for the various target > > If it was some other type, sure, one could use int32plus, lp64, etc. > target, or #if __SIZEOF_type__ == ... I suggest the latter (#if). Ok with that change. Thanks, Richard.
On Fri, Apr 25, 2014 at 10:59:19AM +0200, Richard Biener wrote: > On Fri, 25 Apr 2014, Jakub Jelinek wrote: > > > On Thu, Apr 24, 2014 at 09:20:50PM -0600, Jeff Law wrote: > > > > PR tree-optimization/60930 > > > > * gcc.dg/torture/pr60930.c: New test. > > > Doesn't the test depend on long long being at least 64 bits? > > > > But that is guaranteed by C99, isn't it? > > But the testcase isn't built with -std=c99. It could, but it really doesn't matter. For C89 we provide long long as an extension on all targets, and I'm not aware of any target supported by GCC where long long type precision would be different between C89 and C99 mode, that would be an ABI nightmare. AVR has a non-default -mint8 option that makes it C incompatible (e.g. 8-bit int, 16-bit long and 32-bit long long), but I guess nobody sane would try to run the full gcc testsuite with that option, that would break 50% of tests at least. I think we have plenty of testcases which just assume long long is at least 64-bit. Jakub
On Fri, 2014-04-25 at 10:59 +0200, Richard Biener wrote: > On Fri, 25 Apr 2014, Jakub Jelinek wrote: > > > On Thu, Apr 24, 2014 at 09:20:50PM -0600, Jeff Law wrote: > > > > PR tree-optimization/60930 > > > > * gcc.dg/torture/pr60930.c: New test. > > > Doesn't the test depend on long long being at least 64 bits? > > > > But that is guaranteed by C99, isn't it? > > But the testcase isn't built with -std=c99. > > > 5.2.4.2.1 says: > > > > ... Their implementation-defined values shall be equal or greater in magnitude > > (absolute value) to those shown, with the same sign. > > ... > > - maximum value for an object of type unsigned long long int ULLONG_MAX > > 18446744073709551615 // 2 64 − 1 > > > > > > What we've done for these kinds of tests in the past is: > > > > > > if (sizeof (whatever) < needed size) > > > exit (0); > > > > > > Another approach would be to use an effective target test and skip > > > the test if the target doesn't have a suitable long long. Look in > > > testsuite/lib/target-supports.exp for the various target > > > > If it was some other type, sure, one could use int32plus, lp64, etc. > > target, or #if __SIZEOF_type__ == ... > > I suggest the latter (#if). > > Ok with that change. OK, agreed. I'll add the __SIZEOF_LONG_LONG__ check. I'll give it a week before backporting to 4.8 and 4.9. Thanks, Bill > > Thanks, > Richard.
Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 209714) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -1114,15 +1114,18 @@ create_mul_imm_cand (gimple gs, tree base_in, tree X = Y * c ============================ X = (B + i') * (S * c) */ - base = base_cand->base_expr; - index = base_cand->index; temp = tree_to_double_int (base_cand->stride) * tree_to_double_int (stride_in); - stride = double_int_to_tree (TREE_TYPE (stride_in), temp); - ctype = base_cand->cand_type; - if (has_single_use (base_in)) - savings = (base_cand->dead_savings - + stmt_cost (base_cand->cand_stmt, speed)); + if (double_int_fits_to_tree_p (TREE_TYPE (stride_in), temp)) + { + base = base_cand->base_expr; + index = base_cand->index; + stride = double_int_to_tree (TREE_TYPE (stride_in), temp); + ctype = base_cand->cand_type; + if (has_single_use (base_in)) + savings = (base_cand->dead_savings + + stmt_cost (base_cand->cand_stmt, speed)); + } } else if (base_cand->kind == CAND_ADD && integer_onep (base_cand->stride)) { Index: gcc/testsuite/gcc.dg/torture/pr60930.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr60930.c (revision 0) +++ gcc/testsuite/gcc.dg/torture/pr60930.c (working copy) @@ -0,0 +1,20 @@ +/* { dg-do run } */ + +int x = 1; + +__attribute__((noinline, noclone)) void +foo (unsigned long long t) +{ + asm volatile ("" : : "r" (&t)); + if (t == 1) + __builtin_abort (); +} + +int +main () +{ + unsigned long long t = 0xffffffffffffffffULL * (0xffffffffUL * x); + if (t != 0xffffffff00000001ULL) + foo (t);; + return 0; +}