<div dir="ltr">Hi Sergey,<div><br></div><div>1) I expect the fix should be inside getAArch64Cmp.</div><div>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"".</div>
<div><br></div><div>I also noticed we have the following transformation for if condition (x < 0) in back-end,</div><div>stage 1: (x < 0) -> (x >= 0), i.e. (x<0) and invert the targets.</div><div>stage 2: (x >= 0) -> (x > -1). This happens in combine1.</div>
<div>stage 3: (x > -1) -> (x >= 0) in getAArch64Cmp.</div><div><br></div><div>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).</div>
<div><br></div><div>However, as you can see the root cause of generating the comparison with constant 1 is stage 1. This happens inside SelectionDAGBuilder::visitSwitchCase</div><div><br></div><div><div>  // If the lhs block is the next block, invert the condition so that we can</div>
<div>  // fall through to the lhs instead of the rhs block.</div><div>  if (CB.TrueBB == NextBlock) {</div><div>    std::swap(CB.TrueBB, CB.FalseBB);</div><div>    SDValue True = DAG.getConstant(1, Cond.getValueType());</div>
<div>    Cond = DAG.getNode(ISD::XOR, dl, Cond.getValueType(), Cond, True);</div><div>  }</div></div><div><br></div><div>So now I'm wondering how to justify this is always meaningful for AArch64?</div><div><br></div><div>
Thanks,<br></div><div>-Jiangning</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-07-23 23:54 GMT+08:00 Sergey Dmitrouk <span dir="ltr"><<a href="mailto:sdmitrouk@accesssoftek.com" target="_blank">sdmitrouk@accesssoftek.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Basing on the following information from [this post][0] by James Molloy:<br>
<br>
  2. "if (a < 0 && b == c || a > 0 && b == d)" - the first comparison of<br>
  'a' against zero is done twice, when the flag results of the first<br>
  comparison could be used for the second comparison.<br>
<br>
I've made a patch (attached) that removes this extra comparison.  More<br>
complex cases like comparisons with non-zero immediate values or with<br>
registers doesn't seem to be task for a code generator.  Comparing with<br>
zero is quite common, so I seems to be worth adding.<br>
<br>
Please review the patch.  Couldn't find a better place to make the<br>
change, but I'll be happy to adjust the patch if anyone has better ideas.<br>
<br>
Best regards,<br>
Sergey<br>
<br>
0: <a href="http://article.gmane.org/gmane.comp.compilers.llvm.devel/74269" target="_blank">http://article.gmane.org/gmane.comp.compilers.llvm.devel/74269</a><br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>