* [PATCH] Fix PR60930
@ 2014-04-24 16:38 Bill Schmidt
2014-04-25 3:43 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Bill Schmidt @ 2014-04-24 16:38 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther
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.
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;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR60930
2014-04-24 16:38 [PATCH] Fix PR60930 Bill Schmidt
@ 2014-04-25 3:43 ` Jeff Law
2014-04-25 6:57 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2014-04-25 3:43 UTC (permalink / raw)
To: Bill Schmidt, gcc-patches; +Cc: rguenther
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR60930
2014-04-25 3:43 ` Jeff Law
@ 2014-04-25 6:57 ` Jakub Jelinek
2014-04-25 9:04 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2014-04-25 6:57 UTC (permalink / raw)
To: Jeff Law; +Cc: Bill Schmidt, gcc-patches, rguenther
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR60930
2014-04-25 6:57 ` Jakub Jelinek
@ 2014-04-25 9:04 ` Richard Biener
2014-04-25 9:16 ` Jakub Jelinek
2014-04-25 14:18 ` Bill Schmidt
0 siblings, 2 replies; 6+ messages in thread
From: Richard Biener @ 2014-04-25 9:04 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, Bill Schmidt, gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1136 bytes --]
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR60930
2014-04-25 9:04 ` Richard Biener
@ 2014-04-25 9:16 ` Jakub Jelinek
2014-04-25 14:18 ` Bill Schmidt
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2014-04-25 9:16 UTC (permalink / raw)
To: Richard Biener; +Cc: Jeff Law, Bill Schmidt, gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR60930
2014-04-25 9:04 ` Richard Biener
2014-04-25 9:16 ` Jakub Jelinek
@ 2014-04-25 14:18 ` Bill Schmidt
1 sibling, 0 replies; 6+ messages in thread
From: Bill Schmidt @ 2014-04-25 14:18 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, Jeff Law, gcc-patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-25 14:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-24 16:38 [PATCH] Fix PR60930 Bill Schmidt
2014-04-25 3:43 ` Jeff Law
2014-04-25 6:57 ` Jakub Jelinek
2014-04-25 9:04 ` Richard Biener
2014-04-25 9:16 ` Jakub Jelinek
2014-04-25 14:18 ` Bill 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).