[llvm-commits] [llvm] r133022 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/X86/shl_undef.ll

Bob Wilson bob.wilson at apple.com
Wed Jun 15 09:03:49 PDT 2011


I'm not convinced, Evan.  It seems to me like "shl undef, 1" should always produce a zero lsb.  SimplifyDemandedBits is certainly assuming that.  The testcase has "shl x, 1" where only the lsb is used, so SimplifyDemandedBits replaces x with undef:

...if (DemandedMask == 0) {
    // Not demanding any bits from Op.
    if (Op.getOpcode() != ISD::UNDEF)
      return TLO.CombineTo(Op, TLO.DAG.getUNDEF(Op.getValueType()));

x86 isel matches "shl undef, 1" to "add undef, undef" but then two different registers are used for the two undef operands, and the result is that the lsb is _not_ zero.

I think Chad's fix is a good solution.  Otherwise, if we really want to assert that any operation with an undef input has an undef result, then SimplifyDemandedBits will have to do something more complicated than just inserting undefs as it does in the code above.

BTW, Chad, regarding the comment in your testcase, TargetLoweringOpt is not a pass.  It's just a struct to hold some information used by optimizations in the DAG combiner.

On Jun 14, 2011, at 11:59 PM, Evan Cheng wrote:

> Hi Chad,
> 
> I'm not sure I understand this. It seems like the test cases are dependent on undefined behavior? I don't think anything can assume shl undef, 1 lsb is zero. Anything arithmetics involving undef should be safely folded into undef. 
> 
> Evan
> 
> On Jun 14, 2011, at 3:29 PM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>> Author: mcrosier
>> Date: Tue Jun 14 17:29:10 2011
>> New Revision: 133022
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=133022&view=rev
>> Log:
>> When pattern matching during instruction selection make sure shl x,1 is not
>> converted to add x,x if x is a undef.  add undef, undef does not guarantee
>> that the resulting low order bit is zero.
>> Fixes <rdar://problem/9453156> and <rdar://problem/9487392>.
>> 
>> Added:
>>   llvm/trunk/test/CodeGen/X86/shl_undef.ll
>> Modified:
>>   llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=133022&r1=133021&r2=133022&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Jun 14 17:29:10 2011
>> @@ -3030,6 +3030,9 @@
>>  // fold (shl x, 0) -> x
>>  if (N1C && N1C->isNullValue())
>>    return N0;
>> +  // fold (shl undef, x) -> 0
>> +  if (N0.getOpcode() == ISD::UNDEF)
>> +    return DAG.getConstant(0, VT);
>>  // if (shl x, c) is known to be zero, return 0
>>  if (DAG.MaskedValueIsZero(SDValue(N, 0),
>>                            APInt::getAllOnesValue(OpSizeInBits)))
>> 
>> Added: llvm/trunk/test/CodeGen/X86/shl_undef.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/shl_undef.ll?rev=133022&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/shl_undef.ll (added)
>> +++ llvm/trunk/test/CodeGen/X86/shl_undef.ll Tue Jun 14 17:29:10 2011
>> @@ -0,0 +1,53 @@
>> +; RUN: llc < %s -O1 -mtriple=i386-apple-darwin | FileCheck %s
>> +;
>> +; Interesting test case where %tmp1220 = xor i32 %tmp862, %tmp592 and
>> +; %tmp1676 = xor i32 %tmp1634, %tmp1530 have zero demanded bits after
>> +; TargetLoweringOpt pass.  These are changed to undef and in turn
>> +; the successor shl(s) become shl undef, 1.  This pattern then matches
>> +; shl x, 1 -> add x, x.  add undef, undef doesn't guarentee the low
>> +; order bit is zero and is incorrect.
>> +;
>> +; See rdar://9453156 and rdar://9487392.
>> +;
>> +
>> +; CHECK-NOT: shl
>> +define i32 @foo(i8* %a0, i32* %a2) nounwind {
>> +entry:
>> +  %tmp0 = alloca i8
>> +  %tmp1 = alloca i32
>> +  store i8 1, i8* %tmp0
>> +  %tmp921.i7845 = load i8* %a0, align 1
>> +  %tmp309 = xor i8 %tmp921.i7845, 104
>> +  %tmp592 = zext i8 %tmp309 to i32
>> +  %tmp862 = xor i32 1293461297, %tmp592
>> +  %tmp1220 = xor i32 %tmp862, %tmp592
>> +  %tmp1506 = shl i32 %tmp1220, 1
>> +  %tmp1530 = sub i32 %tmp592, %tmp1506
>> +  %tmp1557 = sub i32 %tmp1530, 542767629
>> +  %tmp1607 = and i32 %tmp1557, 1
>> +  store i32 %tmp1607, i32* %tmp1
>> +  %tmp1634 = and i32 %tmp1607, 2080309246
>> +  %tmp1676 = xor i32 %tmp1634, %tmp1530
>> +  %tmp1618 = shl i32 %tmp1676, 1
>> +  %tmp1645 = sub i32 %tmp862, %tmp1618
>> +  %tmp1697 = and i32 %tmp1645, 1
>> +  store i32 %tmp1697, i32* %a2
>> +  ret i32 %tmp1607
>> +}
>> +
>> +; CHECK-NOT: shl
>> +; shl undef, 0 -> undef
>> +define i32 @foo2_undef() nounwind {
>> +entry:
>> +  %tmp2 = shl i32 undef, 0;
>> +  ret i32 %tmp2
>> +}
>> +
>> +; CHECK-NOT: shl
>> +; shl undef, x -> 0
>> +define i32 @foo1_undef(i32* %a0) nounwind {
>> +entry:
>> +  %tmp1 = load i32* %a0, align 1
>> +  %tmp2 = shl i32 undef, %tmp1;
>> +  ret i32 %tmp2
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list