Re: Move some bit and binary optimizations in simplify and match

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Oct 13, 2015 at 12:52 PM, Hurugalawadi, Naveen
<Naveen.Hurugalawadi@caviumnetworks.com> wrote:
> Hi Richard,
>
> Thanks for the comments. Sorry, I was confused with handling the const and variable
> together part. Have modified them.
> Also, considered that both (X & Y) can be const or variable in those cases
> for which match patterns have been added.

Both can't be constant as (bit_and INTEGER_CST INTEGER_CST) will have been
simplified to a INTEGER_CST already.  Likewise (bit_not INTEGER_CST) will
never appear (that is the problem we are trying to solve!).

> Please let me know whether its correct or only "Y" should be both const and variable
> whereas the "X" should be variable always.
>
> Please find attached the patch as per your comments.
> Please review the patch and let me know if any further modifications
> are required.
>
> Am learning lots of useful stuff while porting these patches.
> Thanks for all the help again.
>
>>> Looks like I really need to make 'match' handle these kind of things.
> I assume that its for bit ops, and binary operations like (A & B) and so on.
> Should I try doing that part? Also, how do we know which patterns should
> be const or variable or supports both?

I was thinking about this for quite a while and didn't find a good solution on
how to implement this reliably other than basically doing the pattern
duplication
in genmatch.  Say, for

/* Fold (A & ~B) - (A & B) into (A ^ B) - B.  */
(simplify
 (minus (bit_and:s @0 (bit_not @1)) (bit_and:s @0 @1))
  (if (! FLOAT_TYPE_P (type))
   (minus (bit_xor @0 @1) @1)))

generate also

(simplify
 (minus (bit_and:s @0 INTEGER_CST@2) (bit_and:s @0 INTEGER_CST@1))
 (if (! FLOAT_TYPE_P (type)
      && wi::eq (const_unop (BIT_NOT_EXPR, @2), @1))
  (minus (bit_xor @0 @1) @1)))

where we'd only target matches and unary ops of depth 1.

The question is whether this is really worth the effort, writing the
above explicitely
isn't too difficult.  So for your case simply do that duplication manually
(not using const_unop of course but wi:: functionality).  Sorry that I misled
you into doing this with (match (xdivamulminusa ..., etc.).  We want to minimize
the number of lines in match.pd and this doesn't really achieve this compared
to duplicating the whole pattern.

Also please take Marcs review comments into account.

+/* Fold (C1/X)*C2 into (C1*C2)/X.  */
+(simplify
+ (mult (rdiv 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)))))

this one is ok with :s added on the rdiv

+/* Simplify ~X & X as zero.  */
+(simplify
+ (bit_and:c (convert? @0) (convert? (bit_not @0)))
+  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
+  { build_zero_cst (TREE_TYPE (@0)); }))

this one is ok as well.

+/* (-A) * (-B) -> A * B  */
+(simplify
+ (mult:c (convert? (negate @0)) (convert? negate_expr_p@1))
+  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
+   (mult (convert @0) (convert (negate @1)))))

this one is ok with using convert1? and convert2?

Please consider splitting those three patterns out (with the suggested
adjustments and the corresponding fold-const.c changes) and committing
them separately to make the rest of the patch smaller.

Thanks,
Richard.



> Thanks,
> Naveen