<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>
> -; rdar://12755626<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>