This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/3] Add simplify rules for wrapped binary operations.
On Tue, 13 Aug 2019, Robin Dapp wrote:
diff --git a/gcc/match.pd b/gcc/match.pd
index 0317bc704f7..94400529ad8 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2020,6 +2020,107 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (cst && !TREE_OVERFLOW (cst))
(plus { cst; } @0))))
+/* ((T)(A + CST1)) + CST2 -> (T)(A) + CST */
+#if GIMPLE
+ (simplify
+ (plus (convert (plus @0 INTEGER_CST@1)) INTEGER_CST@2)
+ (if (INTEGRAL_TYPE_P (type)
+ && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0)))
+ /* We actually want to perform two simplifications here:
Maybe two "transformations"? Since (1) is the exact opposite of the other
simplification you are adding... It would have been nice if it could
indeed be split into 2 simplifications, in particular because we already
have a better (?) version of (2) not far above in the file. But we cannot
add (1) and I can't think of a good way to reuse the existing version of
(2) :-(
+ (1) (T)(A + CST1) + CST2 --> (T)(A) + (T)(CST1)
+ If for (A + CST1) we either do not care about overflow (e.g.
+ for a signed inner type) or the overflow is ok for an unsigned
+ inner type.
From the code, it seems like the condition is really "If we know that no
overflow can occur, either because it would be UB (signed type), or thanks
to range information". Maybe that's the meaning of what you wrote, but I
had trouble interpreting it.
+ (2) (T)(A) + (T)(CST1) + CST2 --> (T)(A) + (T)(CST1 + CST2)
+ If (CST1 + CST2) does not overflow and we do care about overflow
+ (for a signed outer type) or we do not care about overflow in an
+ unsigned outer type. */
+ (with
+ {
+ tree inner_type = TREE_TYPE (@0);
+ wide_int wmin0, wmax0;
+ wide_int cst1 = wi::to_wide (@1);
+
+ wi::overflow_type min_ovf = wi::OVF_OVERFLOW,
There seems to be an inconsistency between spaces and TABs depending on
the lines.
+ max_ovf = wi::OVF_OVERFLOW;
+
+ /* Get overflow behavior. */
+ bool ovf_undef_inner = TYPE_OVERFLOW_UNDEFINED (inner_type);
+ bool ovf_undef_outer = TYPE_OVERFLOW_UNDEFINED (type);
+
+ /* Get value range of A. */
+ enum value_range_kind vr0 = get_range_info (@0, &wmin0, &wmax0);
+
+ /* If we have a proper range, determine min and max overflow
+ of (A + CST1).
At some point we may want to split this into a helper function
operation_may_overflow or something, that takes an arbitrary mix of
SSA_NAME and constants. Not necessary for this patch though.
+ ??? We might also want to handle anti ranges. */
+ if (vr0 == VR_RANGE)
+ {
+ wi::add (wmin0, cst1, TYPE_SIGN (inner_type), &min_ovf);
+ wi::add (wmax0, cst1, TYPE_SIGN (inner_type), &max_ovf);
+ }
+
+ /* Inner overflow does not matter in this case. */
+ if (ovf_undef_inner)
+ {
+ min_ovf = wi::OVF_NONE;
+ max_ovf = wi::OVF_NONE;
+ }
Seems that you could switch the conditions, to avoid unnecessary additions
if (ovf_undef_inner) ... else if (vr0 == VR_RANGE) ...
Maybe even move get_range_info then.
+
+ /* Extend CST from INNER_TYPE to TYPE. */
+ cst1 = cst1.from (cst1, TYPE_PRECISION (type), TYPE_SIGN (inner_type));
wide_int::from? It looks strange to use an arbitrary variable to access a
static function, but that's a style preference, it is ok as is.
+
+ /* Check for overflow of (TYPE)(CST1 + CST2). */
+ wi::overflow_type outer_ovf = wi::OVF_OVERFLOW;
+ wide_int cst = wi::add (cst1, wi::to_wide (@2), TYPE_SIGN (type),
+ &outer_ovf);
+
+ /* We *do* care about an overflow here as we do not want to introduce
+ new undefined behavior that was not there before. */
There is always the possibility to cast to an unsigned type, perform the
operation, then cast back, like the "other (2)" does. But that makes the
transformation even longer, and can lose some information.
+ if (ovf_undef_outer && outer_ovf)
+ {
+ /* Set these here to prevent the final conversion below
+ to take place instead of introducing a new guard variable. */
+ min_ovf = wi::OVF_OVERFLOW;
+ max_ovf = wi::OVF_OVERFLOW;
+ }
+ }
+ (if (min_ovf == wi::OVF_NONE && max_ovf == wi::OVF_NONE)
+ (plus (convert @0) { wide_int_to_tree (type, cst); }
+ )))))
+#endif
+
+/* ((T)(A)) + CST -> (T)(A + CST) */
The other transformation is a true simplification. This one is more a
canonicalization. It does make sense to perform the operation in the
narrower type, I just hope we have enough machinery to revert this later
for platforms that do not like arithmetic in sub-word size. Or probably
this is still restricted enough (doesn't narrow all operations) that it
doesn't matter.
+#if GIMPLE
+ (simplify
+ (plus (convert SSA_NAME@0) INTEGER_CST@1)
+ (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && INTEGRAL_TYPE_P (type)
+ && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))
+ && int_fits_type_p (@1, TREE_TYPE (@0)))
+ /* Perform binary operation inside the cast if the constant fits
+ and (A + CST)'s range does not overflow. */
+ (with
+ {
+ wi::overflow_type min_ovf = wi::OVF_OVERFLOW,
+ max_ovf = wi::OVF_OVERFLOW;
+ tree inner_type = TREE_TYPE (@0);
+
+ wide_int w1 = w1.from (wi::to_wide (@1), TYPE_PRECISION (inner_type),
+ TYPE_SIGN (inner_type));
+
+ wide_int wmin0, wmax0;
+ if (get_range_info (@0, &wmin0, &wmax0) == VR_RANGE)
+ {
+ wi::add (wmin0, w1, TYPE_SIGN (inner_type), &min_ovf);
+ wi::add (wmax0, w1, TYPE_SIGN (inner_type), &max_ovf);
+ }
+ }
+ (if (min_ovf == wi::OVF_NONE && max_ovf == wi::OVF_NONE)
+ (convert (plus @0 { {wide_int_to_tree (TREE_TYPE (@0), w1)}; })))
There are some extra {} here.
+ )))
+#endif
+
--
Marc Glisse