[llvm] r203469 - Fix regression with -O0 for mips .

[Reed Kotler]

On 03/10/2014 10:27 AM, Eric Christopher wrote:
> What kind of regression is this? From your testcase it looks like it
> might not have compiled before? If so, why is adding an optimizer pass
> before regalloc the correct fix?
>
> -eric
>
Sorry for not explaining this in the commit message. There was a bug 
filed against this and it was more completely explained there.

http://llvm.org/bugs/show_bug.cgi?id=19069

The original problem that the optimize pass was created for is a problem 
that occurs with pic when lazy binding is involved.
It shows up in an extremely acute form with mips16 hard float but can 
occur with Mips32 also.

What happens is that if the first time something is called through the 
GOT, the compiler may save the address of the routine in the GOT ; but 
that routine is meant to only be called once and do the resolution and 
then replace itself with the address of the loaded routine. But the 
compiler was saving away the address of the fixup routine so then it 
keeps doing the fixup up many many times which can be in a loop.
(This was also a gcc bug a long time ago when lazy binding was 
introduced for mips).

So the LLVM compiler was changed to optimize this but the change 
requires assumes that this pass is always being called now for one of 
the basic ABI requirements of Mips; which is that the pic call be of the 
form JALR $25.

If this pass is not called, then any register can be used and that will 
break the ABI.

Our pass was added to a the optimize SSA pass which the pass manager 
will not call if -O0 is specified.

The next place in pass manager after the optimize SSA pass is register 
allocation; so by adding this to pre register allocation in the case of 
-O0, all is good again.

Without this; -O0 is broken.
I've run all of test-suite at -O0 with the patch and it works fine and 
we are adding -O0 test-suite runs to our nightly build bots so we don't 
let something like this slip through again.

:)


Reed
> On Mon, Mar 10, 2014 at 9:31 AM, Reed Kotler <rkotler-8NJIiSa5LzA at public.gmane.org> wrote:
>> Author: rkotler
>> Date: Mon Mar 10 11:31:25 2014
>> New Revision: 203469
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=203469&view=rev
>> Log:
>> Fix regression with -O0 for mips .
>>
>>
>> Added:
>>      llvm/trunk/test/CodeGen/Mips/optimize-pic-o0.ll
>> Modified:
>>      llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
>>
>> Modified: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp?rev=203469&r1=203468&r2=203469&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp (original)
>> +++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp Mon Mar 10 11:31:25 2014
>> @@ -175,6 +175,9 @@ public:
>>     virtual bool addInstSelector();
>>     virtual void addMachineSSAOptimization();
>>     virtual bool addPreEmitPass();
>> +
>> +  virtual bool addPreRegAlloc();
>> +
>>   };
>>   } // namespace
>>
>> @@ -208,6 +211,15 @@ void MipsPassConfig::addMachineSSAOptimi
>>     TargetPassConfig::addMachineSSAOptimization();
>>   }
>>
>> +bool MipsPassConfig::addPreRegAlloc() {
>> +  if (getOptLevel() == CodeGenOpt::None) {
>> +    addPass(createMipsOptimizePICCallPass(getMipsTargetMachine()));
>> +    return true;
>> +  }
>> +  else
>> +    return false;
>> +}
>> +
>>   void MipsTargetMachine::addAnalysisPasses(PassManagerBase &PM) {
>>     if (Subtarget.allowMixed16_32()) {
>>       DEBUG(errs() << "No ");
>>
>> Added: llvm/trunk/test/CodeGen/Mips/optimize-pic-o0.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/optimize-pic-o0.ll?rev=203469&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/Mips/optimize-pic-o0.ll (added)
>> +++ llvm/trunk/test/CodeGen/Mips/optimize-pic-o0.ll Mon Mar 10 11:31:25 2014
>> @@ -0,0 +1,33 @@
>> +; RUN: llc -mtriple=mipsel -O0 < %s | FileCheck %s
>> +
>> +; Function Attrs: nounwind
>> +define i32 @main()  {
>> +entry:
>> +  %retval = alloca i32, align 4
>> +  %i = alloca i32, align 4
>> +  store i32 0, i32* %retval
>> +  store i32 0, i32* %i, align 4
>> +  br label %for.cond
>> +
>> +for.cond:                                         ; preds = %for.inc, %entry
>> +  %0 = load i32* %i, align 4
>> +  %cmp = icmp slt i32 %0, 10
>> +  br i1 %cmp, label %for.body, label %for.end
>> +
>> +for.body:                                         ; preds = %for.cond
>> +  call void bitcast (void (...)* @foo to void ()*)()
>> +; CHECK: jalr $25
>> +  br label %for.inc
>> +
>> +for.inc:                                          ; preds = %for.body
>> +  %1 = load i32* %i, align 4
>> +  %inc = add nsw i32 %1, 1
>> +  store i32 %inc, i32* %i, align 4
>> +  br label %for.cond
>> +
>> +for.end:                                          ; preds = %for.cond
>> +  %2 = load i32* %retval
>> +  ret i32 %2
>> +}
>> +
>> +declare void @foo(...)
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits-Tmj1lob9twqVc3sceRu5cw at public.gmane.org
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits