Hi Sergei, Andy,<div><br></div><div>Sorry - I got distracted with some other work. I'm looking into this and PR13719 now. I'll let you know what I find out.</div><div><br></div><div>Sergei - thanks very much for the investigation. That should help me pin this down.</div>
<div><br></div><div>Cheers,</div><div>Lang.<div><br><br><div class="gmail_quote">On Tue, Aug 28, 2012 at 2:33 PM, Sergei Larin <span dir="ltr"><<a href="mailto:slarin@codeaurora.org" target="_blank">slarin@codeaurora.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Andy, Lang,<br>
<br>
Thanks for the suggestion.<br>
<br>
<br>
I have spent more time with it today, and I do see some strange things in<br>
liveness update. I am not at the actual cause yet, but here is what I got so<br>
far:<br>
<br>
I have the following live ranges when I start scheduling a region:<br>
<br>
R2 = [0B,48r:0)[352r,416r:5)...<br>
R3 = [0B,48r:0)[368r,416r:5)...<br>
R4 = [0B,32r:0)[384r,416r:4)...<br>
R5 = [0B,32r:0)[400r,416r:4)...<br>
<br>
I schedule the following instruction (48B):<br>
<br>
0B BB#0: derived from LLVM BB %entry<br>
Live Ins: %R0 %R1 %D1 %D2<br>
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27<br>
12B %vreg30<def> = LDriw <fi#-1>, 0;<br>
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30<br>
20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2]<br>
IntRegs:%vreg31<br>
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26<br>
28B %vreg106<def> = TFRI 16777216;<br>
IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<< CurrentTop<br>
32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29<br>
48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28<br>
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Needs to move above 28B<br>
96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8]<br>
IntRegs:%vreg37<br>
<br>
In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc.<br>
The MI move triggers liveness update, which first triggers SlotIndex<br>
renumbering:<br>
<br>
*** Renumbered SlotIndexes 24-56 ***<br>
<br>
So my 48B becomes 56B, so after the update new live ranges look like this:<br>
<br>
R2 = [0B,56r:0)[352r,416r:5)...<br>
R3 = [0B,56r:0)[368r,416r:5)...<br>
R4 = [0B,48r:0)[384r,416r:4)...<br>
R5 = [0B,48r:0)[400r,416r:4)...<br>
<br>
Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B (also new<br>
after renumbering. But happens to match another old one).<br>
collectRanges for MI figures that it is moving a paired register, and<br>
correctly(?) selects these two ranges to update for %R2:R3<br>
<br>
[0B,56r:0)[368r,416r:5)...<br>
[0B,56r:0)[352r,416r:5)...<br>
<br>
___BUT____ after the update, my new ranges look like this:<br>
<br>
R2 = [0B,32r:0)[352r,416r:5)...<br>
R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have become 48r<br>
R4 = [0B,48r:0)[384r,416r:4)...<br>
R5 = [0B,48r:0)[400r,416r:4)...<br>
....<br>
0B BB#0: derived from LLVM BB %entry<br>
Live Ins: %R0 %R1 %D1 %D2<br>
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27<br>
12B %vreg30<def> = LDriw <fi#-1>, 0;<br>
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30<br>
20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2]<br>
IntRegs:%vreg31<br>
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26<br>
32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28<br>
<<<<<<<<<<<<<<<< Moved instruction<br>
40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106<br>
48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29<br>
96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8]<br>
IntRegs:%vreg37<br>
<br>
This is not caught at this time, and only much later, when another<br>
instruction is scheduled to __the same slot___ the old one "occupied" (48B),<br>
the discrepancy is caught by one of unrelated asserts... I think at that<br>
time there are simply some stale aliases in liveness table.<br>
<br>
I'm going to continue with this tomorrow, but if this helps to identify a<br>
lurking bug today, my day was worth it :) :) :)<br>
<div><br>
Sergei<br>
<br>
--<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.<br>
<br>
<br>
> -----Original Message-----<br>
</div><div><div>> From: Andrew Trick [mailto:<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>]<br>
> Sent: Tuesday, August 28, 2012 3:47 PM<br>
> To: Sergei Larin<br>
> Cc: LLVM Developers Mailing List; Lang Hames<br>
> Subject: Re: [LLVMdev] Assert in LiveInterval update<br>
><br>
> On Aug 28, 2012, at 8:18 AM, Sergei Larin <<a href="mailto:slarin@codeaurora.org" target="_blank">slarin@codeaurora.org</a>><br>
> wrote:<br>
> ><br>
> > I've described that issue (see below) when you were out of town... I<br>
> > think I am getting more context on it. Please take a look...<br>
> ><br>
> > So, in short, when the new MI scheduler performs move of an<br>
> > instruction, it does something like this:<br>
> ><br>
> > // Move the instruction to its new location in the instruction<br>
> stream.<br>
> > MachineInstr *MI = SU->getInstr();<br>
> ><br>
> > if (IsTopNode) {<br>
> > assert(SU->isTopReady() && "node still has unscheduled<br>
> dependencies");<br>
> > if (&*CurrentTop == MI) <<<<<<<<<<<<<<<<<< Here we<br>
> make<br>
> > sure that CurrentTop != MI.<br>
> > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom);<br>
> > else {<br>
> > moveInstruction(MI, CurrentTop);<br>
> > TopRPTracker.setPos(MI);<br>
> > }<br>
> > ...<br>
> ><br>
> > But in moveInstruction we use<br>
> ><br>
> > // Update the instruction stream.<br>
> > BB->splice(InsertPos, BB, MI);<br>
> ><br>
> > And splice as far as I understand moves MI to the location right<br>
> > __before__ InsertPos. Our previous check made sure that InsertPos !=<br>
> > MI, But I do hit a case, when MI == InsertPos--, and effectively<br>
> tries<br>
> > to swap instruction with itself... which make live update mechanism<br>
> very unhappy.<br>
> ><br>
> > If I am missing something in intended logic, please explain,<br>
> otherwise<br>
> > it looks like we need to adjust that check to make sure we never even<br>
> > considering this situation (swapping with self).<br>
> ><br>
> > I also wonder if we want explicit check in live update with more<br>
> > meaningful assert :)<br>
><br>
> Thanks for debugging this! The code above assumes that you're moving an<br>
> unscheduled instruction, which should never be above InsertPos for top-<br>
> down scheduling. If the instruction was in multiple ready Q's, then you<br>
> may attempt to schedule it multiple times. You can avoid this by<br>
> checking Su->isScheduled in your Strategy's pickNode. See<br>
> InstructionShuffler::pickNode for an example. I don't see an equivalent<br>
> check in ConvergingScheduler, but there probably should be.<br>
><br>
> Another possibility to consider is something strange with DebugValues,<br>
> which I haven't tested much.<br>
><br>
> I reproduced the same assert on arm and filed PR13719. I'm not sure yet<br>
> if it's exactly the same issue, but we can move the discussion there.<br>
><br>
> We need a better assert in live update and probably the scheduler too.<br>
> Lang mentioned he may look at the issue today. Meanwhile, I hope my<br>
> suggestion above helps.<br>
><br>
> -Andy<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
</div></div></blockquote></div><br></div></div>