[llvm-commits] [llvm] r62834 - /llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp

[Chris Lattner]

On Jan 22, 2009, at 7:28 PM, Owen Anderson wrote:

> Author: resistor
> Date: Thu Jan 22 21:28:53 2009
> New Revision: 62834
>
> URL: http://llvm.org/viewvc/llvm-project?rev=62834&view=rev
> Log:
> Stage two of fixing pre-alloc-splitting's code size issues: filter  
> out restores that are just
> going to be re-spilled again.
>
> This also helps performance.  Pre-alloc-splitting now seems to be an  
> overall win on SPEC.

nice.

> +      if (VNUseCount[CurrVN].size() == 0) {
> +        LIs->RemoveMachineInstrFromMaps(DefMI);
> +        (*LI)->removeValNo(CurrVN);
> +        DefMI->eraseFromParent();
> +        NumDeadSpills++;
> +        changed = true;

Please use 'continue' here.  This will allow the else to be unnested  
by one level.

>
> +      } else {
> +        bool NonRestore = false;
> +        for (SmallPtrSet<MachineInstr*, 4>::iterator UI =
> +             VNUseCount[CurrVN].begin(), UE =  
> VNUseCount[CurrVN].end();
> +             UI != UI; ++UI) {
> +          int StoreFrameIndex;
> +          unsigned StoreVReg = TII->isStoreToStackSlot(*UI,  
> StoreFrameIndex);
> +          if (StoreVReg != (*LI)->reg || StoreFrameIndex !=  
> FrameIndex) {
> +            NonRestore = false;
> +            break;
> +          }
> +        }

This for loop should be split out to a helper function.  One trivial  
rule is that whenever you need a bool value in your loop like  
"NonRestore", that you should use a helper function.

-Chris