<div class="gmail_quote"><div>Hi Jakob,</div><div><br></div><div>Thanks for the suggestions. They've been applied in r114798.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div><div>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.</div></div></div></blockquote>
<div><br></div><div>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. </div>
<div><br></div><div>Cheers,</div><div>Lang. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div style="word-wrap:break-word"><div><div></div><blockquote type="cite">
<div class="im"><div>Modified: llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp</div></div><div><div class="im">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp?rev=114791&r1=114790&r2=114791&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp?rev=114791&r1=114790&r2=114791&view=diff</a><br>
==============================================================================<br>--- llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/PreAllocSplitting.cpp Sat Sep 25 07:04:16 2010<br>
</div><div class="im">@@ -967,8 +968,7 @@<br><br> assert(!ValNo->isUnused() && "Val# is defined by a dead def?");<br><br>- MachineInstr *DefMI = ValNo->isDefAccurate()<br>- ? LIs->getInstructionFromIndex(ValNo->def) : NULL;<br>
+ MachineInstr *DefMI = LIs->getInstructionFromIndex(ValNo->def);<br><br> // If this would create a new join point, do not split.<br> if (DefMI && createsNewJoin(LR, DefMI->getParent(), Barrier->getParent())) {<br>
@@ -1005,7 +1005,7 @@<br> SlotIndex SpillIndex;<br> MachineInstr *SpillMI = NULL;<br> int SS = -1;<br>- if (!ValNo->isDefAccurate()) {<br>+ if (LIs->getInstructionFromIndex(ValNo->def) == 0) {<br></div></div>
</blockquote><div><br></div><div>This lookup is already available as DefMI above.</div><div class="im"><div><br></div><blockquote type="cite"><div>Modified: llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp</div><div>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp?rev=114791&r1=114790&r2=114791&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp?rev=114791&r1=114790&r2=114791&view=diff</a><br>
==============================================================================<br>--- llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/SimpleRegisterCoalescing.cpp Sat Sep 25 07:04:16 2010<br>
@@ -218,7 +218,7 @@<br> continue;<br> LiveInterval &SRLI = li_->getInterval(*SR);<br> SRLI.addRange(LiveRange(FillerStart, FillerEnd,<br>- SRLI.getNextValue(FillerStart, 0, true,<br>
+ SRLI.getNextValue(FillerStart, 0,<br> li_->getVNInfoAllocator())));<br> }<br> }<br>@@ -267,7 +267,8 @@<br> if (BI->valno == BValNo)<br>
continue;<br> // When BValNo is null, we're looking for a dummy clobber-value for a subreg.<br>- if (!BValNo && !BI->valno->isDefAccurate() && !BI->valno->getCopy())<br>
+ if (!BValNo && li_->getInstructionFromIndex(BI->valno->def) == 0 &&<br>+ !BI->valno->getCopy())<br></div></blockquote><div><br></div></div><div>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.</div>
<div><div></div><div class="h5"><br><blockquote type="cite"><div> continue;<br> if (BI->start <= AI->start && BI->end > AI->start)<br> return true;<br></div></blockquote><div>
<br></div><br><blockquote type="cite"><div>@@ -351,12 +352,11 @@<br> assert(ALR != IntA.end() && "Live range not found!");<br> VNInfo *AValNo = ALR->valno;<br> // If other defs can reach uses of this def, then it's not safe to perform<br>
- // the optimization. FIXME: Do isPHIDef and isDefAccurate both need to be<br>- // tested?<br>- if (AValNo->isPHIDef() || !AValNo->isDefAccurate() ||<br>- AValNo->isUnused() || AValNo->hasPHIKill())<br>
- return false;<br>+ // the optimization.<br> MachineInstr *DefMI = li_->getInstructionFromIndex(AValNo->def);<br>+ if (AValNo->isPHIDef() || DefMI == 0 || AValNo->isUnused() ||<br>+ AValNo->hasPHIKill())<br>
+ return false;<br> if (!DefMI)<br> return false;<br> const TargetInstrDesc &TID = DefMI->getDesc();<br>@@ -652,9 +652,8 @@<br> assert(SrcLR != SrcInt.end() && "Live range not found!");<br>
VNInfo *ValNo = SrcLR->valno;<br> // If other defs can reach uses of this def, then it's not safe to perform<br>- // the optimization. FIXME: Do isPHIDef and isDefAccurate both need to be<br>- // tested?<br>
- if (ValNo->isPHIDef() || !ValNo->isDefAccurate() ||<br>+ // the optimization.<br>+ if (ValNo->isPHIDef() || li_->getInstructionFromIndex(ValNo->def)==0 ||<br> ValNo->isUnused() || ValNo->hasPHIKill())<br>
return false;<br> MachineInstr *DefMI = li_->getInstructionFromIndex(ValNo->def);<font color="#000000"><font color="#144FAE"><br></font></font></div></blockquote><div><br></div></div></div><div>These are both meant to be short-circuit optimizations: if isPHIDef() or isUnused(), don't pay the price of getInstructionFromIndex().</div>
<div><br></div><div>They should read:</div><div><br></div><div>if (isPHIDef || isUnused || hasPHIKill) return false;</div><div class="im"><div>MachineInstr *DefMI = li_->getInstructionFromIndex(ValNo->def);</div></div>
<div>if (!DefMI) return false;</div><div><br></div><font color="#888888"><div>/jakob</div><div><br></div></font></div></div></blockquote></div><br>