On Mon, 17 Feb 2020, Jakub Jelinek wrote:
> On Mon, Feb 17, 2020 at 09:01:13AM +0100, Richard Biener wrote:
> > On Mon, 17 Feb 2020, Li Jia He wrote:
> > > This patch wants to fix PR66552 on gimple and optimizes (x shift (n mod
> > > C))
> > > to (x shift (n bit_and (C - 1))) when C is a constant and power of two as
> > > discussed in PR66552.
> > >
> > > The regression testing for the patch was done on GCC mainline on
> > >
> > > powerpc64le-unknown-linux-gnu (Power 9 LE)
> > >
> > > with no regressions. Is it OK for GCC11 ?
> >
> > I fail to see the connection with a shift operation, can you explain?
>
> As mentioned in the PR, the reason why we can only optimize unsigned
> mod C into bit_and C-1 (where C is pow2p) is that signed modulo can be
> negative for
> negative values (is non-positive). For shifts negative values would be UB
> and so we can assume it will not be negative (i.e. x will be either positive
> or a negative multiple of C) and can use bit_and then.
That's clearly information that should be in the comment before the
pattern. We could also generally do the transform if we know
the other operand of the modulo is nonnegative.
Also the pattern doing the standalone transform does
/* Optimize TRUNC_MOD_EXPR by a power of two into a BIT_AND_EXPR,
i.e. "X % C" into "X & (C - 1)", if X and C are positive.
Also optimize A % (C << N) where C is a power of 2,
to A & ((C << N) - 1). */
(match (power_of_two_cand @1)
INTEGER_CST@1)
(match (power_of_two_cand @1)
(lshift INTEGER_CST@1 @2))
(for mod (trunc_mod floor_mod)
(simplify
(mod @0 (convert?@3 (power_of_two_cand@1 @2)))
(if ((TYPE_UNSIGNED (type)
|| tree_expr_nonnegative_p (@0))
&& tree_nop_conversion_p (type, TREE_TYPE (@3))
&& integer_pow2p (@2) && tree_int_cst_sgn (@2) > 0)
(bit_and @0 (convert (minus @1 { build_int_cst (TREE_TYPE (@1), 1);
}))))))
so it also checks whether @2 is not negative, the new pattern lacks
this and could make use of power_of_two_cand as well (in fact I'd
place it next to the pattern above.
Also the above applies for trunc_mod and floor_mod but the proposed
patch applies the transform for ceil_mod and round_mod as well.
Richard.
