[llvm-commits] [llvm] r114791 - in /llvm/trunk: include/llvm/CodeGen/LiveInterval.h lib/CodeGen/InlineSpiller.cpp lib/CodeGen/LiveInterval.cpp lib/CodeGen/LiveIntervalAnalysis.cpp lib/CodeGen/PreAllocSplitting.cpp lib/CodeGen/RegAllocLinearScan.cpp lib/CodeGen/RegAllocPBQP.cpp lib/CodeGen/SimpleRegisterCoalescing.cpp lib/CodeGen/Spiller.cpp lib/CodeGen/SplitKit.cpp lib/CodeGen/Splitter.cpp

[Jakob Stoklund Olesen]

On Sep 25, 2010, at 5:04 AM, Lang Hames wrote:

> Author: lhames
> Date: Sat Sep 25 07:04:16 2010
> New Revision: 114791
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=114791&view=rev
> Log:
> Removed VNInfo::isDefAccurate(). Def "accuracy" can be checked by testing whether LiveIntervals::getInstructionFromIndex(def) returns NULL.

Awesome. Thanks, Lang!

> Modified: llvm/trunk/lib/CodeGen/InlineSpiller.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/InlineSpiller.cpp?rev=114791&r1=114790&r2=114791&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/InlineSpiller.cpp (original)
> +++ llvm/trunk/lib/CodeGen/InlineSpiller.cpp Sat Sep 25 07:04:16 2010
> @@ -283,7 +282,7 @@
>     lis_.RemoveMachineInstrFromMaps(DefMI);
>     vrm_.RemoveMachineInstrFromMaps(DefMI);
>     DefMI->eraseFromParent();
> -    VNI->setIsDefAccurate(false);
> +    VNI->def = lis_.getZeroIndex();

Would it be possible to simply use SlotIndex() here? I am not completely clear on the difference, but it would be nice to have just one exceptional SlotIndex.

> Modified: llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp?rev=114791&r1=114790&r2=114791&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp (original)
> +++ llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp Sat Sep 25 07:04:16 2010
> @@ -967,8 +968,7 @@
> 
>   assert(!ValNo->isUnused() && "Val# is defined by a dead def?");
> 
> -  MachineInstr *DefMI = ValNo->isDefAccurate()
> -    ? LIs->getInstructionFromIndex(ValNo->def) : NULL;
> +  MachineInstr *DefMI = LIs->getInstructionFromIndex(ValNo->def);
> 
>   // If this would create a new join point, do not split.
>   if (DefMI && createsNewJoin(LR, DefMI->getParent(), Barrier->getParent())) {
> @@ -1005,7 +1005,7 @@
>   SlotIndex SpillIndex;
>   MachineInstr *SpillMI = NULL;
>   int SS = -1;
> -  if (!ValNo->isDefAccurate()) {
> +  if (LIs->getInstructionFromIndex(ValNo->def) == 0) {

This lookup is already available as DefMI above.

> Modified: llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp?rev=114791&r1=114790&r2=114791&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp Sat Sep 25 07:04:16 2010
> @@ -218,7 +218,7 @@
>         continue;
>       LiveInterval &SRLI = li_->getInterval(*SR);
>       SRLI.addRange(LiveRange(FillerStart, FillerEnd,
> -                              SRLI.getNextValue(FillerStart, 0, true,
> +                              SRLI.getNextValue(FillerStart, 0,
>                                                 li_->getVNInfoAllocator())));
>     }
>   }
> @@ -267,7 +267,8 @@
>       if (BI->valno == BValNo)
>         continue;
>       // When BValNo is null, we're looking for a dummy clobber-value for a subreg.
> -      if (!BValNo && !BI->valno->isDefAccurate() && !BI->valno->getCopy())
> +      if (!BValNo && li_->getInstructionFromIndex(BI->valno->def) == 0 &&
> +          !BI->valno->getCopy())

Yeah, this is my fault. Those dummy clobber values don't exist any more. They used inaccurate defs a lot. This test can just be assumed false always.

>         continue;
>       if (BI->start <= AI->start && BI->end > AI->start)
>         return true;


> @@ -351,12 +352,11 @@
>   assert(ALR != IntA.end() && "Live range not found!");
>   VNInfo *AValNo = ALR->valno;
>   // If other defs can reach uses of this def, then it's not safe to perform
> -  // the optimization. FIXME: Do isPHIDef and isDefAccurate both need to be
> -  // tested?
> -  if (AValNo->isPHIDef() || !AValNo->isDefAccurate() ||
> -      AValNo->isUnused() || AValNo->hasPHIKill())
> -    return false;
> +  // the optimization.
>   MachineInstr *DefMI = li_->getInstructionFromIndex(AValNo->def);
> +  if (AValNo->isPHIDef() || DefMI == 0 || AValNo->isUnused() ||
> +      AValNo->hasPHIKill())
> +    return false;
>   if (!DefMI)
>     return false;
>   const TargetInstrDesc &TID = DefMI->getDesc();
> @@ -652,9 +652,8 @@
>   assert(SrcLR != SrcInt.end() && "Live range not found!");
>   VNInfo *ValNo = SrcLR->valno;
>   // If other defs can reach uses of this def, then it's not safe to perform
> -  // the optimization. FIXME: Do isPHIDef and isDefAccurate both need to be
> -  // tested?
> -  if (ValNo->isPHIDef() || !ValNo->isDefAccurate() ||
> +  // the optimization.
> +  if (ValNo->isPHIDef() || li_->getInstructionFromIndex(ValNo->def)==0 ||
>       ValNo->isUnused() || ValNo->hasPHIKill())
>     return false;
>   MachineInstr *DefMI = li_->getInstructionFromIndex(ValNo->def);

These are both meant to be short-circuit optimizations: if isPHIDef() or isUnused(), don't pay the price of getInstructionFromIndex().

They should read:

if (isPHIDef || isUnused || hasPHIKill) return false;
MachineInstr *DefMI = li_->getInstructionFromIndex(ValNo->def);
if (!DefMI) return false;

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100925/658cc252/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1929 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100925/658cc252/attachment.bin>