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

[Evan Cheng]

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. 

Evan

Sent from my iPad

On May 13, 2013, at 4:01 PM, David Majnemer <david.majnemer at gmail.com> wrote:

> 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/e86d7772/attachment.html>