[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.c

[Lang Hames]

Hi Jakob,

Thanks for the suggestions. They've been applied in r114798.


> 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.
>

LiveIntervals::getZeroIndex() returns an index which will compare lower than
any other (and is the valid first index in the function). SlotIndex() values
will compare higher than any other. Whether any code actually relies on this
property I am not sure. SlotIndex() values are _not_ valid slots for any
function, and it's impossible to traverse from them to any valid slot.
They're self contained "weird" values. For that reason I think SlotIndex()
would be better as the exceptional value.

Cheers,
Lang.

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/20100926/0e04c871/attachment.html>