[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 --- Comment #10 from Richard Biener --- Author: rguenth Date: Mon Feb 1 15:40:23 2016 New Revision: 233040 URL: https://gcc.gnu.org/viewcvs?rev=233040&root=gcc&view=rev Log: 2016-02-01 Richard Biener PR middle-end/69556 * match.pd: Guard (C1/X)*C2 -> (C1*C2)/X with single_use. * gcc.dg/tree-ssa/recip-8.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/tree-ssa/recip-8.c Modified: trunk/gcc/ChangeLog trunk/gcc/match.pd trunk/gcc/testsuite/ChangeLog
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #9 from Richard Biener --- Fixed.
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556
--- Comment #8 from Richard Biener ---
Testcase that we'd break with removing the canonicalization for the
non-single-use case:
int bar (double, double);
int
foo (double a)
{
double x = 2./a;
double y = 4./a;
return x * 2. == y || bar (x, y);
}
note that the transform is guarded with a different flag than the reverse
done by the recip pass (-fassociative-math vs. -freciprocal-math). So it
isn't a canonicalization (which we should be able to always undo).
That said, before moving the pattern it was effectively only applied to
the single-use (if GIMPLE optimizers didn't feed fold with enough IL to
trigger it).
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 Richard Biener changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org --- Comment #7 from Richard Biener --- Mine.
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 --- Comment #6 from rguenther at suse dot de --- On January 29, 2016 10:45:12 PM GMT+01:00, "glisse at gcc dot gnu.org" wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 > >--- Comment #5 from Marc Glisse --- >(In reply to rguent...@suse.de from comment #4) >> I think there is a misunderstanding. A replacement is still allowed >if it >> is a single operation as that replaces at least one other (the one we >are >> simplifying). This assumes equal cost of course which for divide vs. >Mult >> is not the case. So an explicit && single_use as in the patch below >is >> needed. > >The number of patterns that have to use an explicit single_use is >growing, >maybe we need a syntax like :S for "single_use, and I mean it, not like >:s". Heh. I hoped to avoid this and find some better way to cater to the different users of the machinery in the gcc 7 timeframe.
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 --- Comment #5 from Marc Glisse --- (In reply to rguent...@suse.de from comment #4) > I think there is a misunderstanding. A replacement is still allowed if it > is a single operation as that replaces at least one other (the one we are > simplifying). This assumes equal cost of course which for divide vs. Mult > is not the case. So an explicit && single_use as in the patch below is > needed. The number of patterns that have to use an explicit single_use is growing, maybe we need a syntax like :S for "single_use, and I mean it, not like :s".
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556
--- Comment #4 from rguenther at suse dot de ---
On January 29, 2016 6:39:07 PM GMT+01:00, "jgreenhalgh at gcc dot gnu.org"
wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556
>
>James Greenhalgh changed:
>
> What|Removed |Added
>
>CC||jgreenhalgh at gcc dot gnu.org,
> ||rguenth at gcc dot gnu.org
>
>--- Comment #3 from James Greenhalgh
>---
>(In reply to Andrew Pinski from comment #2)
>> (In reply to Andrew Pinski from comment #1)
>> > I suspect we should disable "Fold (C1/X)*C2 into (C1*C2)/X" for
>gimple then
>> > and have it only for generic.
>>
>> Or check for single use of the divide.
>
>I had thought that was what the :s in the first line of pattern was
>trying to
>do:
>
> (simplify
> (mult (rdiv:s REAL_CST@0 @1) REAL_CST@2)
>(if (flag_associative_math)
> (with
> { tree tem = const_binop (MULT_EXPR, type, @0, @2); }
> (if (tem)
> (rdiv { tem; } @1)
>
>If I capture the rdiv, and explicitly check it for single_use (as in
>the
>untested patch below), then the rule fails. So there's either a
>misunderstanding/disagreement here about what :s implies, or the
>match.pd
>machinery has a bug.
I think there is a misunderstanding. A replacement is still allowed if it is a
single operation as that replaces at least one other (the one we are
simplifying). This assumes equal cost of course which for divide vs. Mult is
not the case. So an explicit && single_use as in the patch below is needed.
Note that this will disable CSE of the two cases as well then.
Richard.
>diff --git a/gcc/match.pd b/gcc/match.pd
>index 5f28215..9460a9b 100644
>--- a/gcc/match.pd
>+++ b/gcc/match.pd
>@@ -445,11 +445,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
> /* Fold (C1/X)*C2 into (C1*C2)/X. */
> (simplify
>- (mult (rdiv:s REAL_CST@0 @1) REAL_CST@2)
>+ (mult (rdiv:s@3 REAL_CST@0 @1) REAL_CST@2)
> (if (flag_associative_math)
>(with
> { tree tem = const_binop (MULT_EXPR, type, @0, @2); }
>-(if (tem)
>+(if (tem && single_use (@3))
> (rdiv { tem; } @1)
>
> /* Convert C1/(X*C2) into (C1/C2)/X */
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556
James Greenhalgh changed:
What|Removed |Added
CC||jgreenhalgh at gcc dot gnu.org,
||rguenth at gcc dot gnu.org
--- Comment #3 from James Greenhalgh ---
(In reply to Andrew Pinski from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I suspect we should disable "Fold (C1/X)*C2 into (C1*C2)/X" for gimple then
> > and have it only for generic.
>
> Or check for single use of the divide.
I had thought that was what the :s in the first line of pattern was trying to
do:
(simplify
(mult (rdiv:s REAL_CST@0 @1) REAL_CST@2)
(if (flag_associative_math)
(with
{ tree tem = const_binop (MULT_EXPR, type, @0, @2); }
(if (tem)
(rdiv { tem; } @1)
If I capture the rdiv, and explicitly check it for single_use (as in the
untested patch below), then the rule fails. So there's either a
misunderstanding/disagreement here about what :s implies, or the match.pd
machinery has a bug.
diff --git a/gcc/match.pd b/gcc/match.pd
index 5f28215..9460a9b 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -445,11 +445,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Fold (C1/X)*C2 into (C1*C2)/X. */
(simplify
- (mult (rdiv:s REAL_CST@0 @1) REAL_CST@2)
+ (mult (rdiv:s@3 REAL_CST@0 @1) REAL_CST@2)
(if (flag_associative_math)
(with
{ tree tem = const_binop (MULT_EXPR, type, @0, @2); }
-(if (tem)
+(if (tem && single_use (@3))
(rdiv { tem; } @1)
/* Convert C1/(X*C2) into (C1/C2)/X */
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 --- Comment #2 from Andrew Pinski --- (In reply to Andrew Pinski from comment #1) > I suspect we should disable "Fold (C1/X)*C2 into (C1*C2)/X" for gimple then > and have it only for generic. Or check for single use of the divide.
[Bug tree-optimization/69556] [6 Regression] forwprop4/match.pd undoing work from recip
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69556 Andrew Pinski changed: What|Removed |Added Keywords||missed-optimization Status|UNCONFIRMED |NEW Last reconfirmed||2016-01-29 CC||pinskia at gcc dot gnu.org Target Milestone|--- |6.0 Ever confirmed|0 |1 --- Comment #1 from Andrew Pinski --- I suspect we should disable "Fold (C1/X)*C2 into (C1*C2)/X" for gimple then and have it only for generic.
