[llvm] r204435 - [Constant Hoisting] Make the constant materialization cost operand dependent

Wed Apr 2 18:11:07 PDT 2014

On Mar 31, 2014, at 4:48 PM, Eric Christopher <echristo at gmail.com> wrote:

>> 
>> +  unsigned ImmIdx = ~0U;
>>   switch (Opcode) {
>> +  default: return TCC_Free;
>> +  case Instruction::GetElementPtr:
>> +    if (Idx != 0)
>> +      return TCC_Free;
>> +  case Instruction::Store:
>> +    ImmIdx = 0;
>> +    break;
> 
> I remember you fixed a fallthrough in a followup patch but I can't
> recall if you fixed up this one or the one here:

This was an intended fall-though, but that got replaced in r204739.

> 
>>   case Intrinsic::experimental_stackmap:
>> +    if (Idx < 2)
>> +      return TCC_Free;
>>   case Intrinsic::experimental_patchpoint_void:
> 
> I think it was the first and if so, did you mean the fallthrough here?

This was the think-o that got fixed in r204738. It wasn’t meant to fall-through.

> 
> 
> 
>> 
>> Modified: llvm/trunk/test/CodeGen/X86/lsr-interesting-step.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/lsr-interesting-step.ll?rev=204435&r1=204434&r2=204435&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/lsr-interesting-step.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/lsr-interesting-step.ll Fri Mar 21 01:04:45 2014
>> @@ -3,26 +3,24 @@
>> ; The inner loop should require only one add (and no leas either).
>> ; rdar://8100380
>> 
>> -; CHECK:      BB0_3:
>> -; CHECK-NEXT:   movb    $0, flags(%rdx)
>> -; CHECK-NEXT:   addq    %rax, %rdx
>> -; CHECK-NEXT:   cmpq    $8192, %rdx
>> +; CHECK:      BB0_2:
>> +; CHECK-NEXT:   movb    $0, flags(%rcx)
>> +; CHECK-NEXT:   addq    %rax, %rcx
>> +; CHECK-NEXT:   cmpq    $8192, %rcx
>> ; CHECK-NEXT:   jl
>> 
>> @flags = external global [8192 x i8], align 16 ; <[8192 x i8]*> [#uses=1]
>> 
>> define void @foo() nounwind {
>> entry:
>> -  %tmp = icmp slt i64 2, 8192                     ; <i1> [#uses=1]
>> -  br i1 %tmp, label %bb, label %bb21
>> +  br label %bb
>> 
>> bb:                                               ; preds = %entry
>>   br label %bb7
>> 
>> bb7:                                              ; preds = %bb, %bb17
>>   %tmp8 = phi i64 [ %tmp18, %bb17 ], [ 2, %bb ]   ; <i64> [#uses=2]
>> -  %tmp9 = icmp slt i64 2, 8192                    ; <i1> [#uses=1]
>> -  br i1 %tmp9, label %bb10, label %bb17
>> +  br label %bb10
>> 
> 
> Why did the actual testcase change here?

The test started to fail with the operand aware cost model. Constant hoisting would start hoisting the first operand of the icmp and
the CHECKs wouldn’t match the new output. I updated the CHECKs and manually constant folded some instructions away. 

> 
>> Modified: llvm/trunk/test/CodeGen/X86/negate-add-zero.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/negate-add-zero.ll?rev=204435&r1=204434&r2=204435&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/negate-add-zero.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/negate-add-zero.ll Fri Mar 21 01:04:45 2014
>> @@ -827,9 +827,7 @@ declare void @_ZN11MatrixTools9transpose
>> declare void @_ZN21HNodeTranslateRotate311toCartesianEv(%struct.HNodeTranslateRotate3*)
>> 
>> define linkonce void @_ZN21HNodeTranslateRotate36setVelERK9CDSVectorIdLi1EN3CDS12DefaultAllocEE(%struct.HNodeTranslateRotate3* %this, %"struct.CDSVector<double,0,CDS::DefaultAlloc>"* %velv) {
>> -entry:
>> -       %0 = add i32 0, -1              ; <i32> [#uses=1]
>> -       %1 = getelementptr double* null, i32 %0         ; <double*> [#uses=1]
>> +       %1 = getelementptr double* null, i32 -1         ; <double*> [#uses=1]
>>        %2 = load double* %1, align 8           ; <double> [#uses=1]
>>        %3 = load double* null, align 8         ; <double> [#uses=2]
>>        %4 = load double* null, align 8         ; <double> [#uses=2]
>> @@ -890,13 +888,12 @@ entry:
>>        store double %52, double* %55, align 8
>>        %56 = getelementptr %struct.HNodeTranslateRotate3* %this, i32 0, i32 0, i32 10, i32 0, i32 0, i32 2             ; <double*> [#uses=1]
>>        store double %53, double* %56, align 8
>> -       %57 = add i32 0, 4              ; <i32> [#uses=1]
>> -       %58 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 0           ; <%"struct.CDSVector<double,0,CDS::DefaultAlloc>"**> [#uses=1]
>> -       store %"struct.CDSVector<double,0,CDS::DefaultAlloc>"* %velv, %"struct.CDSVector<double,0,CDS::DefaultAlloc>"** %58, align 8
>> -       %59 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 1           ; <i32*> [#uses=1]
>> -       store i32 %57, i32* %59, align 4
>> -       %60 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 2           ; <i32*> [#uses=1]
>> -       store i32 3, i32* %60, align 8
>> +       %57 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 0           ; <%"struct.CDSVector<double,0,CDS::DefaultAlloc>"**> [#uses=1]
>> +       store %"struct.CDSVector<double,0,CDS::DefaultAlloc>"* %velv, %"struct.CDSVector<double,0,CDS::DefaultAlloc>"** %57, align 8
>> +       %58 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 1           ; <i32*> [#uses=1]
>> +       store i32 4, i32* %58, align 4
>> +       %59 = getelementptr %"struct.SubVector<CDSVector<double, 1, CDS::DefaultAlloc> >"* null, i32 0, i32 2           ; <i32*> [#uses=1]
>> +       store i32 3, i32* %59, align 8
>>        unreachable
>> }
>> 
> 
> Ditto.

Ditto

> 
>> 
>> Modified: llvm/trunk/test/Transforms/ConstantHoisting/X86/phi.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ConstantHoisting/X86/phi.ll?rev=204435&r1=204434&r2=204435&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/ConstantHoisting/X86/phi.ll (original)
>> +++ llvm/trunk/test/Transforms/ConstantHoisting/X86/phi.ll Fri Mar 21 01:04:45 2014
>> @@ -20,10 +20,10 @@ return:
>> 
>> ; CHECK-LABEL: @test1
>> ; CHECK: if.end:
>> -; CHECK: %const_mat = add i64 %const, 1
>> -; CHECK-NEXT: %1 = inttoptr i64 %const_mat to i8*
>> +; CHECK: %2 = inttoptr i64 %const to i8*
>> +; CHECK-NEXT: br
>> ; CHECK: return:
>> -; CHECK-NEXT: %retval.0 = phi i8* [ null, %entry ], [ inttoptr (i64 68719476736 to i8*), %if.end ]
>> +; CHECK-NEXT: %retval.0 = phi i8* [ null, %entry ], [ %2, %if.end ]
>> }
>> 
> 
> This looks like a nice result though. :)
> 
> -eric