[PATCH][AArch64] Enable sign check optimization by CSE

[Jiangning Liu]

Hi Sergey,

1) I expect the fix should be inside getAArch64Cmp.
2) Your comment is inconsistent with your code. Your code is to transform
(x < 1) to be (x<=0), rather than "Turn "x > 1" condition into "x >= 0"".

I also noticed we have the following transformation for if condition (x <
0) in back-end,
stage 1: (x < 0) -> (x >= 0), i.e. (x<0) and invert the targets.
stage 2: (x >= 0) -> (x > -1). This happens in combine1.
stage 3: (x > -1) -> (x >= 0) in getAArch64Cmp.

For if condition (x > 0), the transformation is similar. Your patch is
trying to cover this case, but essentially getAArch64Cmp missed the case of
(x < 1) -> (x <= 0).

However, as you can see the root cause of generating the comparison with
constant 1 is stage 1. This happens
inside SelectionDAGBuilder::visitSwitchCase

  // If the lhs block is the next block, invert the condition so that we can
  // fall through to the lhs instead of the rhs block.
  if (CB.TrueBB == NextBlock) {
    std::swap(CB.TrueBB, CB.FalseBB);
    SDValue True = DAG.getConstant(1, Cond.getValueType());
    Cond = DAG.getNode(ISD::XOR, dl, Cond.getValueType(), Cond, True);
  }

So now I'm wondering how to justify this is always meaningful for AArch64?

Thanks,
-Jiangning


2014-07-23 23:54 GMT+08:00 Sergey Dmitrouk <sdmitrouk at accesssoftek.com>:

> Hi,
>
> Basing on the following information from [this post][0] by James Molloy:
>
>   2. "if (a < 0 && b == c || a > 0 && b == d)" - the first comparison of
>   'a' against zero is done twice, when the flag results of the first
>   comparison could be used for the second comparison.
>
> I've made a patch (attached) that removes this extra comparison.  More
> complex cases like comparisons with non-zero immediate values or with
> registers doesn't seem to be task for a code generator.  Comparing with
> zero is quite common, so I seems to be worth adding.
>
> Please review the patch.  Couldn't find a better place to make the
> change, but I'll be happy to adjust the patch if anyone has better ideas.
>
> Best regards,
> Sergey
>
> 0: http://article.gmane.org/gmane.comp.compilers.llvm.devel/74269
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140724/468860dd/attachment.html>