This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Reassociate X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0


On 10/10/13 05:12, Jakub Jelinek wrote:
On Mon, Aug 05, 2013 at 04:08:58PM +0800, Zhenqiang Chen wrote:
ChangeLog
2013-08-05  Zhenqiang Chen  <zhenqiang.chen@arm.com>

	* tree-ssa-reassoc.c (optimize_range_tests): Reasociate
	X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into
	((X - CST1) & ~(CST2 - CST1)) == 0.

testsuite/ChangeLog
2013-08-05  Zhenqiang Chen  <zhenqiang.chen@arm.com>

	* gcc.dg/reassoc1.c: New testcase.

+  /* Optimize X == CST1 || X == CST2
+     if popcount (CST2 - CST1) == 1 into
+     ((X - CST1) & ~(CST2 - CST1)) == 0.  */
+  if (!any_changes && (opcode == BIT_IOR_EXPR))

Wouldn't it be better to put it into the same loop that handles
the other merges, rather than separate?  Why the !any_changes
guard?  Why opcode == BIT_IOR_EXPR guard?  Can't you optimize
X != CST1 && X != CST2 if popcount (CST2 - CST1) == 1
similarly into ((X - CST1) & ~(CST2 - CST1)) != 0?

Also, like the preexisting optimization has been generalized for ranges,
can't you generalize this one too?

I mean if you have say:
(X - 43U) <= 3U || (X - 75U) <= 3U
(originally e.g. X == 43 || X == 76 || X == 44 || X == 78 || X == 77 || X == 46 || X == 75 || X == 45)
where popcount (75U - 43U) == 1, can't this be:
((X - 43U) & ~(75U - 43U)) <= 3U

For X in [LOWCST1, HIGHCST1] || X in [LOWCST2, HIGHCST2]
the requirements for this optimization would be
1) LOWCST2 > HIGHCST1
2) HIGHCST1 - LOWCST1 == HIGHCST2 - LOWCST2
3) popcount (LOWCST2 - LOWCST1) == 1

1) and 2) are the same requirements for the other
optimization, while 3) would be specific to this
and would be used only if popcount (LOWCST1 ^ LOWCST2) != 1.
Because of 1), LOWCST2 - LOWCST1 should be bigger than
HIGHCST1 - LOWCST1 (i.e. the value we <= against in the end),
thus IMHO it should work fine even after generalization.

+	    if (tree_log2 (tem) < 0)
+	      continue;

This is I guess cheaper than what I was doing there:
           gcc_checking_assert (!integer_zerop (lowxor));
           tem = fold_binary (MINUS_EXPR, type, lowxor,
                              build_int_cst (type, 1));
           if (tem == NULL_TREE)
             continue;
           tem = fold_binary (BIT_AND_EXPR, type, lowxor, tem);
           if (tem == NULL_TREE || !integer_zerop (tem))
             continue;
to check for popcount (x) == 1.  Can you replace it even in the
preexisting code?
	if (tree_log2 (lowxor) < 0)
	  continue;
?
Of course, if the two optimizations are merged into the same loop,
then some of the continue will need to be replaced by just conditional
code inside of the loop, handling the two different optimizations.

Thanks for working on this.
Clearly you should be the one reviewing this patch Jakub ;-) I'm handing it off to you.


jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]