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

[Manman Ren]

On Dec 21, 2012, at 6:27 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Tue, Dec 11, 2012 at 5:13 PM, Manman Ren <mren at apple.com> wrote:
>> Author: mren
>> Date: Tue Dec 11 19:13:50 2012
>> New Revision: 169951
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=169951&view=rev
>> Log:
>> DAGCombine: clamp hi bit in APInt::getBitsSet to avoid assertion
>> 
>> rdar://12838504
>> 
>> Modified:
>>    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>    llvm/trunk/test/CodeGen/X86/store_op_load_fold.ll
>> 
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=169951&r1=169950&r2=169951&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Dec 11 19:13:50 2012
>> @@ -7433,7 +7433,8 @@
>>     // start at the previous one.
>>     if (ShAmt % NewBW)
>>       ShAmt = (((ShAmt + NewBW - 1) / NewBW) * NewBW) - NewBW;
>> -    APInt Mask = APInt::getBitsSet(BitWidth, ShAmt, ShAmt + NewBW);
>> +    APInt Mask = APInt::getBitsSet(BitWidth, ShAmt,
>> +                                   std::min(BitWidth, ShAmt + NewBW));
>>     if ((Imm & Mask) == Imm) {
>>       APInt NewImm = (Imm & Mask).lshr(ShAmt).trunc(NewBW);
>>       if (Opc == ISD::AND)
> 
> Are you sure the math here is actually correct?  This looks really
> suspicious, especially without a comment explaining how the math is
> supposed to work.
Hi Eli,

Sorry for my late response. My change was to clamp the high bit of Mask to not exceed the actual APInt's high bit.
Is that incorrect?

Thanks,
Manman
> 
> -Eli
> 
>> Modified: llvm/trunk/test/CodeGen/X86/store_op_load_fold.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/store_op_load_fold.ll?rev=169951&r1=169950&r2=169951&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/store_op_load_fold.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/store_op_load_fold.ll Tue Dec 11 19:13:50 2012
>> @@ -1,13 +1,30 @@
>> -; RUN: llc < %s -march=x86 | not grep mov
>> +; RUN: llc < %s -march=x86 | FileCheck %s
>> ;
>> ; Test the add and load are folded into the store instruction.
>> 
>> @X = internal global i16 0              ; <i16*> [#uses=2]
>> 
>> define void @foo() nounwind {
>> +; CHECK: foo:
>> +; CHECK-NOT: mov
>> +; CHECK: add
>> +; CHECK_NEXT: ret
>>         %tmp.0 = load i16* @X           ; <i16> [#uses=1]
>>         %tmp.3 = add i16 %tmp.0, 329            ; <i16> [#uses=1]
>>         store i16 %tmp.3, i16* @X
>>         ret void
>> }
>> 
>> +; rdar://12838504
>> +%struct.S2 = type { i64, i16, [2 x i8], i8, [3 x i8], [7 x i8], i8, [8 x i8] }
>> + at s2 = external global %struct.S2, align 16
>> +define void @test2() nounwind uwtable ssp {
>> +; CHECK: test2:
>> +; CHECK: mov
>> +; CHECK-NEXT: and
>> +; CHECK-NEXT: ret
>> +  %bf.load35 = load i56* bitcast ([7 x i8]* getelementptr inbounds (%struct.S2* @s2, i32 0, i32 5) to i56*), align 16
>> +  %bf.clear36 = and i56 %bf.load35, -1125895611875329
>> +  store i56 %bf.clear36, i56* bitcast ([7 x i8]* getelementptr inbounds (%struct.S2* @s2, i32 0, i32 5) to i56*), align 16
>> +  ret void
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits