On Thu, 13 Jul 2017, Alexander Monakov wrote:
This is a followup to https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01545.html
Recently due to a fix for PR 80800 GCC has lost the ability to reassociate
signed multiplications chains to go from 'X * CST1 * Y * CST2'
to 'X * Y * (CST1 * CST2)'. The fix to that PR prevents extract_muldiv from
introducing '(X * (CST1 * CST2)) * Y', which was wrong because it may cause
intermediate signed overflow (unexpected if Y == 0).
As mentioned in that thread, we can reassociate constants to outermost operands
instead: this is safe because CST1 cannot be 0 or -1, since those are handled
by other match.pd rules.
(in fact it's possible to reassociate negates too, and go from '(-X) * Y * CST'
to '(X * Y) * (-CST)' if (-CST) doesn't overflow (again, we know that CST != 1),
but I'm not sure how valuable that is in practice, so opted not to do that yet)
The following patch reinstates folding by adding a new match.pd rule that moves
constants to outermost operands, where they can be merged by fold_binary
machinery if their product doesn't overflow. Bootstrapped and regtested on
amd64, OK for trunk?
* match.pd ((X * CST) * Y): Reassociate to (X * Y) * CST.
diff --git a/gcc/match.pd b/gcc/match.pd
index 4c64b21..e49f879 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2139,6 +2139,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(mult @0 integer_minus_onep)
(negate @0))
+/* Reassociate (X * CST) * Y to (X * Y) * CST. This does not introduce
+ signed overflow: previous rules handle CST being -1 or 0, and for
+ the rest we know that if X * Y overflows, so does (X * CST) * Y. */
+(if (!TYPE_OVERFLOW_SANITIZED (type) && !TYPE_SATURATING (type))
+ (simplify
+ (mult:c (mult @0 INTEGER_CST@1) @2)
+ (if (TREE_CODE (@2) != INTEGER_CST)
+ (mult (mult @0 @2) @1))))
+
/* True if we can easily extract the real and imaginary parts of a complex
number. */
(match compositional_complex
Thanks. I guess it makes sense as a canonicalization (there are always
cases where those make it harder to optimize, but hopefully fewer than
those where they help).
I notice that we do not turn (X*10)*10 into X*100 in GIMPLE. X*big*big
where abs(big*big)>abs(INT_MIN) can be optimized to 0, the only hard case
is when the product of the constants is -INT_MIN, which we could turn into
X<<31 for instance (sadly loses range info), or (-X)*INT_MIN or whatever.
That would make a nice follow-up, if you are interested.
Relying on inner expressions being folded can be slightly dangerous,
especially for generic IIRC. It seems easy enough to check that @1 is
neither 0 nor -1 for safety.
Probably needs :s on the inner multiplication.
Unless the test on TYPE_OVERFLOW_SANITIZED etc is shared with adjacent
transformations, I'd rather put it inside, with the other if, but that's a
matter of taste.
One small testcase please? Or is there already one that is currently
failing?
(I cannot ok patches, only comment)
--
Marc Glisse
