[llvm] r181145 - Remove a recently redundant transform from X86ISelLowering.

[David Majnemer]

I suppose so but the transform in question is an optimization, the code
still gets lowered correctly without it.

Should I revert this removal?


On Mon, May 13, 2013 at 3:41 PM, Evan Cheng <evan.cheng at apple.com> wrote:

> 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.
>
> Evan
>
> On May 4, 2013, at 7:00 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>
> > Author: majnemer
> > Date: Sat May  4 21:00:10 2013
> > New Revision: 181145
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=181145&view=rev
> > Log:
> > Remove a recently redundant transform from X86ISelLowering.
> >
> > X86ISelLowering has support to treat:
> > (icmp ne (and (xor %flags, -1), (shl 1, flag)), 0)
> >
> > as if it were actually:
> > (icmp eq (and %flags, (shl 1, flag)), 0)
> >
> > However, r179386 has code at the InstCombine level to handle this.
> >
> > Modified:
> >    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >    llvm/trunk/test/CodeGen/X86/bt.ll
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=181145&r1=181144&r2=181145&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat May  4 21:00:10
> 2013
> > @@ -9180,14 +9180,6 @@ SDValue X86TargetLowering::LowerToBT(SDV
> >   }
> >
> >   if (LHS.getNode()) {
> > -    // If the LHS is of the form (x ^ -1) then replace the LHS with x
> and flip
> > -    // the condition code later.
> > -    bool Invert = false;
> > -    if (LHS.getOpcode() == ISD::XOR && isAllOnes(LHS.getOperand(1))) {
> > -      Invert = true;
> > -      LHS = LHS.getOperand(0);
> > -    }
> > -
> >     // If LHS is i8, promote it to i32 with any_extend.  There is no i8
> BT
> >     // instruction.  Since the shift amount is in-range-or-undefined, we
> know
> >     // that doing a bittest on the i32 value is ok.  We extend to i32
> because
> > @@ -9204,9 +9196,6 @@ SDValue X86TargetLowering::LowerToBT(SDV
> >
> >     SDValue BT = DAG.getNode(X86ISD::BT, dl, MVT::i32, LHS, RHS);
> >     X86::CondCode Cond = CC == ISD::SETEQ ? X86::COND_AE : X86::COND_B;
> > -    // Flip the condition if the LHS was a not instruction
> > -    if (Invert)
> > -      Cond = X86::GetOppositeBranchCondition(Cond);
> >     return DAG.getNode(X86ISD::SETCC, dl, MVT::i8,
> >                        DAG.getConstant(Cond, MVT::i8), BT);
> >   }
> >
> > Modified: llvm/trunk/test/CodeGen/X86/bt.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bt.ll?rev=181145&r1=181144&r2=181145&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/bt.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/bt.ll Sat May  4 21:00:10 2013
> > @@ -522,11 +522,8 @@ UnifiedReturnBlock:              ; preds = %entry
> >
> > declare void @foo()
> >
> > -; rdar://12755626
> > define zeroext i1 @invert(i32 %flags, i32 %flag) nounwind {
> > -; CHECK: invert
> > -; CHECK: btl %eax, %ecx
> > -; CHECK: setae
> > +; CHECK: btl
> > entry:
> >   %neg = xor i32 %flags, -1
> >   %shl = shl i32 1, %flag
> >
> >
> > _______________________________________________
> > 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/20130513/b3779c4c/attachment.html>