On 12 October 2016 at 14:43, Richard Biener <rguent...@suse.de> wrote:
> On Wed, 12 Oct 2016, Marc Glisse wrote:
>
>> On Wed, 12 Oct 2016, Prathamesh Kulkarni wrote:
>>
>> > I was having a look at PR71636 and added the following pattern to match.pd:
>> > x & ((1U << b) - 1) -> x & ~(~0U << b)
>> > However the transform is useful only if the target supports "andnot"
>> > instruction.
>>
>> rth was selling the transformation as a canonicalization, which is beneficial
>> when there is an andnot instruction, and neutral otherwise, so it could be
>> done always.
>
> Well, its three instructions to three instructions and a more expensive
> constant(?). ~0U might not be available as immediate for the shift
> instruction and 1U << b might be available as a bit-set instruction ...
> (vs. the andnot).
>
> So yes, we might decide to canonicalize to andnot (and decide that
> three binary to two binary and one unary op is "better").
>
> So no excuse to explore the target specific .pd fragment idea ... :/
Hi,
I have attached patch that adds the transform.
Does that look OK ?
I am not sure how to write test-cases for it though.
For the test-case:
unsigned f(unsigned x, unsigned b)
{
unsigned t1 = 1U << b;
unsigned t2 = t1 - 1;
unsigned t3 = x & t2;
return t3;
}
forwprop dump shows:
Applying pattern match.pd:523, gimple-match.c:47419
gimple_simplified to _6 = 4294967295 << b_1(D);
_8 = ~_6;
t3_5 = x_4(D) & _8;
I could scan for "_6 = 4294967295 << b_1(D);" however I suppose
~0 would depend on width of int and not always be 4294967295 ?
Or should I scan for "_6 = 4294967295 << b_1(D);"
and add /* { dg-require-effective int32 } */ to the test-case ?
Thanks,
Prathamesh
>
> Richard.
>
>> > As pointed out by Marc in PR for -march=core2, lhs generates worse
>> > code than rhs,
>> > so we shouldn't do the transform if target doesn't support andnot insn.
>> > (perhaps we could do the reverse transform for target not supporting
>> > andnot?)
>>
>> Rereading my comment in the PR, I pointed out that instead of being neutral,
>> the transformation was very slightly detrimental in one case (one extra mov)
>> because of a RA issue. That doesn't mean we should avoid the transformation,
>> just that we should fix the RA issue (by the way, if you have time to file a
>> separate PR for the RA issue, that would be great, otherwise I'll try to do
>> it
>> at some point...).
>>
>> > However it seems andnot isn't a standard pattern name, so am not sure how
>> > to
>> > check if target supports andnot insn ?
>
diff --git a/gcc/match.pd b/gcc/match.pd
index 1d80613..742d48f 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -516,6 +516,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(bit_and:c (convert? @0) (convert? (bit_not @0)))
{ build_zero_cst (type); })
+/* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */
+(simplify
+ (bit_and:c @0 (plus (lshift integer_onep @1) integer_minus_onep))
+ (if (TYPE_UNSIGNED (type))
+ (bit_and @0 (bit_not (lshift (bit_not { build_zero_cst (type); }) @1)))))
+
/* Fold (A & ~B) - (A & B) into (A ^ B) - B. */
(simplify
(minus (bit_and:cs @0 (bit_not @1)) (bit_and:cs @0 @1))
