<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>It's not totally obvious to me. This case is not a big deal since it's very specialized transformation. However, I'd like to caution against this practice in the future. We can't always assume clients are running code through the standard optimization passes. </div><div><br></div><div>Evan<br><br>Sent from my iPad</div><div><br>On May 13, 2013, at 4:01 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr">I suppose so but the transform in question is an optimization, the code still gets lowered correctly without it.<div><br></div><div style="">Should I revert this removal?</div></div><div class="gmail_extra"><br>
<br><div class="gmail_quote">On Mon, May 13, 2013 at 3:41 PM, Evan Cheng <span dir="ltr"><<a href="mailto:evan.cheng@apple.com" target="_blank">evan.cheng@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't think that having a transformation in InstCombine is a good argument for removing the transformation in isel. It's possible to have clients of llvm which only uses codegen but not the optimizer.<br>
<span class="HOEnZb"><font color="#888888"><br>
Evan<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On May 4, 2013, at 7:00 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
<br>
> Author: majnemer<br>
> Date: Sat May 4 21:00:10 2013<br>
> New Revision: 181145<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181145&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181145&view=rev</a><br>
> Log:<br>
> Remove a recently redundant transform from X86ISelLowering.<br>
><br>
> X86ISelLowering has support to treat:<br>
> (icmp ne (and (xor %flags, -1), (shl 1, flag)), 0)<br>
><br>
> as if it were actually:<br>
> (icmp eq (and %flags, (shl 1, flag)), 0)<br>
><br>
> However, r179386 has code at the InstCombine level to handle this.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
> llvm/trunk/test/CodeGen/X86/bt.ll<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=181145&r1=181144&r2=181145&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=181145&r1=181144&r2=181145&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat May 4 21:00:10 2013<br>
> @@ -9180,14 +9180,6 @@ SDValue X86TargetLowering::LowerToBT(SDV<br>
> }<br>
><br>
> if (LHS.getNode()) {<br>
> - // If the LHS is of the form (x ^ -1) then replace the LHS with x and flip<br>
> - // the condition code later.<br>
> - bool Invert = false;<br>
> - if (LHS.getOpcode() == ISD::XOR && isAllOnes(LHS.getOperand(1))) {<br>
> - Invert = true;<br>
> - LHS = LHS.getOperand(0);<br>
> - }<br>
> -<br>
> // If LHS is i8, promote it to i32 with any_extend. There is no i8 BT<br>
> // instruction. Since the shift amount is in-range-or-undefined, we know<br>
> // that doing a bittest on the i32 value is ok. We extend to i32 because<br>
> @@ -9204,9 +9196,6 @@ SDValue X86TargetLowering::LowerToBT(SDV<br>
><br>
> SDValue BT = DAG.getNode(X86ISD::BT, dl, MVT::i32, LHS, RHS);<br>
> X86::CondCode Cond = CC == ISD::SETEQ ? X86::COND_AE : X86::COND_B;<br>
> - // Flip the condition if the LHS was a not instruction<br>
> - if (Invert)<br>
> - Cond = X86::GetOppositeBranchCondition(Cond);<br>
> return DAG.getNode(X86ISD::SETCC, dl, MVT::i8,<br>
> DAG.getConstant(Cond, MVT::i8), BT);<br>
> }<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/bt.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bt.ll?rev=181145&r1=181144&r2=181145&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bt.ll?rev=181145&r1=181144&r2=181145&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/bt.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/bt.ll Sat May 4 21:00:10 2013<br>
> @@ -522,11 +522,8 @@ UnifiedReturnBlock: ; preds = %entry<br>
><br>
> declare void @foo()<br>
><br>
> -; <a href="rdar://12755626">rdar://12755626</a><br>
> define zeroext i1 @invert(i32 %flags, i32 %flag) nounwind {<br>
> -; CHECK: invert<br>
> -; CHECK: btl %eax, %ecx<br>
> -; CHECK: setae<br>
> +; CHECK: btl<br>
> entry:<br>
> %neg = xor i32 %flags, -1<br>
> %shl = shl i32 1, %flag<br>
><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>
</div></div></blockquote></div><br></div>
</div></blockquote></body></html>