Andy, Lang, Thanks for the suggestion. I have spent more time with it today, and I do see some strange things in liveness update. I am not at the actual cause yet, but here is what I got so far: I have the following live ranges when I start scheduling a region: R2 = [0B,48r:0)[352r,416r:5)... R3 = [0B,48r:0)[368r,416r:5)... R4 = [0B,32r:0)[384r,416r:4)... R5 = [0B,32r:0)[400r,416r:4)... I schedule the following instruction (48B): 0B BB#0: derived from LLVM BB %entry Live Ins: %R0 %R1 %D1 %D2 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 12B %vreg30<def> = LDriw <fi#-1>, 0; mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] IntRegs:%vreg31 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 28B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<< CurrentTop 32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Needs to move above 28B 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] IntRegs:%vreg37 In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc. The MI move triggers liveness update, which first triggers SlotIndex renumbering: *** Renumbered SlotIndexes 24-56 *** So my 48B becomes 56B, so after the update new live ranges look like this: R2 = [0B,56r:0)[352r,416r:5)... R3 = [0B,56r:0)[368r,416r:5)... R4 = [0B,48r:0)[384r,416r:4)... R5 = [0B,48r:0)[400r,416r:4)... Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B (also new after renumbering. But happens to match another old one). collectRanges for MI figures that it is moving a paired register, and correctly(?) selects these two ranges to update for %R2:R3 [0B,56r:0)[368r,416r:5)... [0B,56r:0)[352r,416r:5)... ___BUT____ after the update, my new ranges look like this: R2 = [0B,32r:0)[352r,416r:5)... R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have become 48r R4 = [0B,48r:0)[384r,416r:4)... R5 = [0B,48r:0)[400r,416r:4)... .... 0B BB#0: derived from LLVM BB %entry Live Ins: %R0 %R1 %D1 %D2 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 12B %vreg30<def> = LDriw <fi#-1>, 0; mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] IntRegs:%vreg31 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 <<<<<<<<<<<<<<<< Moved instruction 40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106 48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] IntRegs:%vreg37 This is not caught at this time, and only much later, when another instruction is scheduled to __the same slot___ the old one "occupied" (48B), the discrepancy is caught by one of unrelated asserts... I think at that time there are simply some stale aliases in liveness table. I'm going to continue with this tomorrow, but if this helps to identify a lurking bug today, my day was worth it :) :) :) Sergei -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.> -----Original Message----- > From: Andrew Trick [mailto:atrick at apple.com] > Sent: Tuesday, August 28, 2012 3:47 PM > To: Sergei Larin > Cc: LLVM Developers Mailing List; Lang Hames > Subject: Re: [LLVMdev] Assert in LiveInterval update > > On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at codeaurora.org> > wrote: > > > > I've described that issue (see below) when you were out of town... I > > think I am getting more context on it. Please take a look... > > > > So, in short, when the new MI scheduler performs move of an > > instruction, it does something like this: > > > > // Move the instruction to its new location in the instruction > stream. > > MachineInstr *MI = SU->getInstr(); > > > > if (IsTopNode) { > > assert(SU->isTopReady() && "node still has unscheduled > dependencies"); > > if (&*CurrentTop == MI) <<<<<<<<<<<<<<<<<< Here we > make > > sure that CurrentTop != MI. > > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom); > > else { > > moveInstruction(MI, CurrentTop); > > TopRPTracker.setPos(MI); > > } > > ... > > > > But in moveInstruction we use > > > > // Update the instruction stream. > > BB->splice(InsertPos, BB, MI); > > > > And splice as far as I understand moves MI to the location right > > __before__ InsertPos. Our previous check made sure that InsertPos !> > MI, But I do hit a case, when MI == InsertPos--, and effectively > tries > > to swap instruction with itself... which make live update mechanism > very unhappy. > > > > If I am missing something in intended logic, please explain, > otherwise > > it looks like we need to adjust that check to make sure we never even > > considering this situation (swapping with self). > > > > I also wonder if we want explicit check in live update with more > > meaningful assert :) > > Thanks for debugging this! The code above assumes that you're moving an > unscheduled instruction, which should never be above InsertPos for top- > down scheduling. If the instruction was in multiple ready Q's, then you > may attempt to schedule it multiple times. You can avoid this by > checking Su->isScheduled in your Strategy's pickNode. See > InstructionShuffler::pickNode for an example. I don't see an equivalent > check in ConvergingScheduler, but there probably should be. > > Another possibility to consider is something strange with DebugValues, > which I haven't tested much. > > I reproduced the same assert on arm and filed PR13719. I'm not sure yet > if it's exactly the same issue, but we can move the discussion there. > > We need a better assert in live update and probably the scheduler too. > Lang mentioned he may look at the issue today. Meanwhile, I hope my > suggestion above helps. > > -Andy
Sergei Larin
2012-Aug-30 19:30 UTC
[LLVMdev] MC Register mapping question (MCRegUnitIterator )
Hello Jakob and everyone,
I am observing an issue with MCRegUnitIterator in my back end, and trying
to reverse engineer some of the table gen magic around it, but if you or
someone readily knows the answer, I would highly appreciate it.
Here is the problem.
In my back end we have a rather simple int register file structure:
// Integer registers.
def R0 : Ri< 0, "r0">, DwarfRegNum<[0]>;
def R1 : Ri< 1, "r1">, DwarfRegNum<[1]>;
def R2 : Ri< 2, "r2">, DwarfRegNum<[2]>;
def R3 : Ri< 3, "r3">, DwarfRegNum<[3]>;
...
...which could be accessed as double regs in pairs:
// Aliases of the R* registers used to hold 64-bit int values (doubles).
let SubRegIndices = [subreg_loreg, subreg_hireg] in {
def D0 : Rd< 0, "r1:0", [R0, R1]>,
DwarfRegNum<[32]>;
def D1 : Rd< 2, "r3:2", [R2, R3]>,
DwarfRegNum<[34]>;
def D2 : Rd< 4, "r5:4", [R4, R5]>,
DwarfRegNum<[36]>;
So R2:R3 are subregs of D1.
These definitions are mapped to HexagonGenRegisterInfo.inc in something like
this:
enum {
NoRegister,
D0 = 1,
D1 = 2,
D2 = 3,
D3 = 4,
D4 = 5,
...
R0 = 27,
R1 = 28,
R2 = 29,
R3 = 30,
R4 = 31,
...
NUM_TARGET_REGS // 62
};
On the other end of the problem, I use the following to iterate over
sub-regs of (in this case) D1:
for (MCRegUnitIterator Units(Reg, &TRI); Units.isValid(); ++Units) {
DEBUG(dbgs() << "\tUnit (" << *Units <<
")\n");
...
}
For Reg==2==D1 I expect to see *Units to be 29==R2 and 30==R3 (its subregs).
Instead I see it to take values of 2==D1 and 3==D2!
Moreover, if I iterate over units of an ordinary register, let's say
Reg==29==R2, *Units above iterates once with value of 2==D1!
Either I do not understand how MCRegUnitIterator should work, or (more
likely) I have a problem in lib/Target/Hexagon/HexagonRegisterInfo.td,
because it seems that in HexagonGenRegisterInfo.inc I got something that
confuses tables to iterate over for subregs...
So, if you know what it might be, please let me know. Thanks a lot.
Sergei Larin
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
> -----Original Message-----
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu]
> On Behalf Of Sergei Larin
> Sent: Tuesday, August 28, 2012 4:34 PM
> To: 'Andrew Trick'
> Cc: 'LLVM Developers Mailing List'
> Subject: Re: [LLVMdev] Assert in LiveInterval update
>
> Andy, Lang,
>
> Thanks for the suggestion.
>
>
> I have spent more time with it today, and I do see some strange
> things in liveness update. I am not at the actual cause yet, but here
> is what I got so
> far:
>
> I have the following live ranges when I start scheduling a region:
>
> R2 = [0B,48r:0)[352r,416r:5)...
> R3 = [0B,48r:0)[368r,416r:5)...
> R4 = [0B,32r:0)[384r,416r:4)...
> R5 = [0B,32r:0)[400r,416r:4)...
>
> I schedule the following instruction (48B):
>
> 0B BB#0: derived from LLVM BB %entry
> Live Ins: %R0 %R1 %D1 %D2
> 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
> 12B %vreg30<def> = LDriw <fi#-1>, 0;
> mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
> 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2]
> IntRegs:%vreg31
> 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
> 28B %vreg106<def> = TFRI 16777216;
>
IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<<
CurrentTop
> 32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
> 48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
>
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Needs to move above 28B
> 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8]
> IntRegs:%vreg37
>
> In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc.
> The MI move triggers liveness update, which first triggers SlotIndex
> renumbering:
>
> *** Renumbered SlotIndexes 24-56 ***
>
> So my 48B becomes 56B, so after the update new live ranges look like
> this:
>
> R2 = [0B,56r:0)[352r,416r:5)...
> R3 = [0B,56r:0)[368r,416r:5)...
> R4 = [0B,48r:0)[384r,416r:4)...
> R5 = [0B,48r:0)[400r,416r:4)...
>
> Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B
> (also new after renumbering. But happens to match another old one).
> collectRanges for MI figures that it is moving a paired register, and
> correctly(?) selects these two ranges to update for %R2:R3
>
> [0B,56r:0)[368r,416r:5)...
> [0B,56r:0)[352r,416r:5)...
>
> ___BUT____ after the update, my new ranges look like this:
>
> R2 = [0B,32r:0)[352r,416r:5)...
> R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should
have
> become 48r
> R4 = [0B,48r:0)[384r,416r:4)...
> R5 = [0B,48r:0)[400r,416r:4)...
> ....
> 0B BB#0: derived from LLVM BB %entry
> Live Ins: %R0 %R1 %D1 %D2
> 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
> 12B %vreg30<def> = LDriw <fi#-1>, 0;
> mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
> 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2]
> IntRegs:%vreg31
> 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
> 32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
> <<<<<<<<<<<<<<<< Moved
instruction
> 40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106
> 48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
> 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8]
> IntRegs:%vreg37
>
> This is not caught at this time, and only much later, when another
> instruction is scheduled to __the same slot___ the old one
"occupied"
> (48B), the discrepancy is caught by one of unrelated asserts... I think
> at that time there are simply some stale aliases in liveness table.
>
> I'm going to continue with this tomorrow, but if this helps to identify
> a lurking bug today, my day was worth it :) :) :)
>
> Sergei
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.
>
>
> > -----Original Message-----
> > From: Andrew Trick [mailto:atrick at apple.com]
> > Sent: Tuesday, August 28, 2012 3:47 PM
> > To: Sergei Larin
> > Cc: LLVM Developers Mailing List; Lang Hames
> > Subject: Re: [LLVMdev] Assert in LiveInterval update
> >
> > On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at
codeaurora.org>
> > wrote:
> > >
> > > I've described that issue (see below) when you were out of
town...
> > > I think I am getting more context on it. Please take a look...
> > >
> > > So, in short, when the new MI scheduler performs move of an
> > > instruction, it does something like this:
> > >
> > > // Move the instruction to its new location in the instruction
> > stream.
> > > MachineInstr *MI = SU->getInstr();
> > >
> > > if (IsTopNode) {
> > > assert(SU->isTopReady() && "node still has
unscheduled
> > dependencies");
> > > if (&*CurrentTop == MI)
<<<<<<<<<<<<<<<<<< Here
> we
> > make
> > > sure that CurrentTop != MI.
> > > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom);
> > > else {
> > > moveInstruction(MI, CurrentTop);
> > > TopRPTracker.setPos(MI);
> > > }
> > > ...
> > >
> > > But in moveInstruction we use
> > >
> > > // Update the instruction stream.
> > > BB->splice(InsertPos, BB, MI);
> > >
> > > And splice as far as I understand moves MI to the location right
> > > __before__ InsertPos. Our previous check made sure that InsertPos
> !> > > MI, But I do hit a case, when MI == InsertPos--, and
effectively
> > tries
> > > to swap instruction with itself... which make live update
mechanism
> > very unhappy.
> > >
> > > If I am missing something in intended logic, please explain,
> > otherwise
> > > it looks like we need to adjust that check to make sure we never
> > > even considering this situation (swapping with self).
> > >
> > > I also wonder if we want explicit check in live update with more
> > > meaningful assert :)
> >
> > Thanks for debugging this! The code above assumes that you're
moving
> > an unscheduled instruction, which should never be above InsertPos for
> > top- down scheduling. If the instruction was in multiple ready
Q's,
> > then you may attempt to schedule it multiple times. You can avoid
> this
> > by checking Su->isScheduled in your Strategy's pickNode. See
> > InstructionShuffler::pickNode for an example. I don't see an
> > equivalent check in ConvergingScheduler, but there probably should
> be.
> >
> > Another possibility to consider is something strange with
> DebugValues,
> > which I haven't tested much.
> >
> > I reproduced the same assert on arm and filed PR13719. I'm not
sure
> > yet if it's exactly the same issue, but we can move the discussion
> there.
> >
> > We need a better assert in live update and probably the scheduler
> too.
> > Lang mentioned he may look at the issue today. Meanwhile, I hope my
> > suggestion above helps.
> >
> > -Andy
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Arnold Schwaighofer
2012-Aug-30 19:43 UTC
[LLVMdev] MC Register mapping question (MCRegUnitIterator )
Hi Sergei, Register units != sub registers. Register units are an abstraction to describe overlapping of registers effectively. You probably wanted to use MCSubRegIterator. On Thu, Aug 30, 2012 at 2:30 PM, Sergei Larin <slarin at codeaurora.org> wrote:> Hello Jakob and everyone, > > I am observing an issue with MCRegUnitIterator in my back end, and trying > to reverse engineer some of the table gen magic around it, but if you or > someone readily knows the answer, I would highly appreciate it. > > Here is the problem. > > In my back end we have a rather simple int register file structure: > > // Integer registers. > def R0 : Ri< 0, "r0">, DwarfRegNum<[0]>; > def R1 : Ri< 1, "r1">, DwarfRegNum<[1]>; > def R2 : Ri< 2, "r2">, DwarfRegNum<[2]>; > def R3 : Ri< 3, "r3">, DwarfRegNum<[3]>; > ... > > ...which could be accessed as double regs in pairs: > > // Aliases of the R* registers used to hold 64-bit int values (doubles). > let SubRegIndices = [subreg_loreg, subreg_hireg] in { > def D0 : Rd< 0, "r1:0", [R0, R1]>, DwarfRegNum<[32]>; > def D1 : Rd< 2, "r3:2", [R2, R3]>, DwarfRegNum<[34]>; > def D2 : Rd< 4, "r5:4", [R4, R5]>, DwarfRegNum<[36]>; > > So R2:R3 are subregs of D1. > > These definitions are mapped to HexagonGenRegisterInfo.inc in something like > this: > enum { > NoRegister, > D0 = 1, > D1 = 2, > D2 = 3, > D3 = 4, > D4 = 5, > ... > R0 = 27, > R1 = 28, > R2 = 29, > R3 = 30, > R4 = 31, > ... > NUM_TARGET_REGS // 62 > }; > > On the other end of the problem, I use the following to iterate over > sub-regs of (in this case) D1: > > for (MCRegUnitIterator Units(Reg, &TRI); Units.isValid(); ++Units) { > DEBUG(dbgs() << "\tUnit (" << *Units << ")\n"); > ... > } > > For Reg==2==D1 I expect to see *Units to be 29==R2 and 30==R3 (its subregs). > Instead I see it to take values of 2==D1 and 3==D2! > > Moreover, if I iterate over units of an ordinary register, let's say > Reg==29==R2, *Units above iterates once with value of 2==D1! > > Either I do not understand how MCRegUnitIterator should work, or (more > likely) I have a problem in lib/Target/Hexagon/HexagonRegisterInfo.td, > because it seems that in HexagonGenRegisterInfo.inc I got something that > confuses tables to iterate over for subregs... > > So, if you know what it might be, please let me know. Thanks a lot. > > Sergei Larin > > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by > The Linux Foundation > > >> -----Original Message----- >> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] >> On Behalf Of Sergei Larin >> Sent: Tuesday, August 28, 2012 4:34 PM >> To: 'Andrew Trick' >> Cc: 'LLVM Developers Mailing List' >> Subject: Re: [LLVMdev] Assert in LiveInterval update >> >> Andy, Lang, >> >> Thanks for the suggestion. >> >> >> I have spent more time with it today, and I do see some strange >> things in liveness update. I am not at the actual cause yet, but here >> is what I got so >> far: >> >> I have the following live ranges when I start scheduling a region: >> >> R2 = [0B,48r:0)[352r,416r:5)... >> R3 = [0B,48r:0)[368r,416r:5)... >> R4 = [0B,32r:0)[384r,416r:4)... >> R5 = [0B,32r:0)[400r,416r:4)... >> >> I schedule the following instruction (48B): >> >> 0B BB#0: derived from LLVM BB %entry >> Live Ins: %R0 %R1 %D1 %D2 >> 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 >> 12B %vreg30<def> = LDriw <fi#-1>, 0; >> mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 >> 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] >> IntRegs:%vreg31 >> 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 >> 28B %vreg106<def> = TFRI 16777216; >> IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<< CurrentTop >> 32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 >> 48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 >> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Needs to move above 28B >> 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] >> IntRegs:%vreg37 >> >> In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc. >> The MI move triggers liveness update, which first triggers SlotIndex >> renumbering: >> >> *** Renumbered SlotIndexes 24-56 *** >> >> So my 48B becomes 56B, so after the update new live ranges look like >> this: >> >> R2 = [0B,56r:0)[352r,416r:5)... >> R3 = [0B,56r:0)[368r,416r:5)... >> R4 = [0B,48r:0)[384r,416r:4)... >> R5 = [0B,48r:0)[400r,416r:4)... >> >> Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B >> (also new after renumbering. But happens to match another old one). >> collectRanges for MI figures that it is moving a paired register, and >> correctly(?) selects these two ranges to update for %R2:R3 >> >> [0B,56r:0)[368r,416r:5)... >> [0B,56r:0)[352r,416r:5)... >> >> ___BUT____ after the update, my new ranges look like this: >> >> R2 = [0B,32r:0)[352r,416r:5)... >> R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have >> become 48r >> R4 = [0B,48r:0)[384r,416r:4)... >> R5 = [0B,48r:0)[400r,416r:4)... >> .... >> 0B BB#0: derived from LLVM BB %entry >> Live Ins: %R0 %R1 %D1 %D2 >> 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 >> 12B %vreg30<def> = LDriw <fi#-1>, 0; >> mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 >> 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] >> IntRegs:%vreg31 >> 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 >> 32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 >> <<<<<<<<<<<<<<<< Moved instruction >> 40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106 >> 48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 >> 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] >> IntRegs:%vreg37 >> >> This is not caught at this time, and only much later, when another >> instruction is scheduled to __the same slot___ the old one "occupied" >> (48B), the discrepancy is caught by one of unrelated asserts... I think >> at that time there are simply some stale aliases in liveness table. >> >> I'm going to continue with this tomorrow, but if this helps to identify >> a lurking bug today, my day was worth it :) :) :) >> >> Sergei >> >> -- >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum. >> >> >> > -----Original Message----- >> > From: Andrew Trick [mailto:atrick at apple.com] >> > Sent: Tuesday, August 28, 2012 3:47 PM >> > To: Sergei Larin >> > Cc: LLVM Developers Mailing List; Lang Hames >> > Subject: Re: [LLVMdev] Assert in LiveInterval update >> > >> > On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at codeaurora.org> >> > wrote: >> > > >> > > I've described that issue (see below) when you were out of town... >> > > I think I am getting more context on it. Please take a look... >> > > >> > > So, in short, when the new MI scheduler performs move of an >> > > instruction, it does something like this: >> > > >> > > // Move the instruction to its new location in the instruction >> > stream. >> > > MachineInstr *MI = SU->getInstr(); >> > > >> > > if (IsTopNode) { >> > > assert(SU->isTopReady() && "node still has unscheduled >> > dependencies"); >> > > if (&*CurrentTop == MI) <<<<<<<<<<<<<<<<<< Here >> we >> > make >> > > sure that CurrentTop != MI. >> > > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom); >> > > else { >> > > moveInstruction(MI, CurrentTop); >> > > TopRPTracker.setPos(MI); >> > > } >> > > ... >> > > >> > > But in moveInstruction we use >> > > >> > > // Update the instruction stream. >> > > BB->splice(InsertPos, BB, MI); >> > > >> > > And splice as far as I understand moves MI to the location right >> > > __before__ InsertPos. Our previous check made sure that InsertPos >> !>> > > MI, But I do hit a case, when MI == InsertPos--, and effectively >> > tries >> > > to swap instruction with itself... which make live update mechanism >> > very unhappy. >> > > >> > > If I am missing something in intended logic, please explain, >> > otherwise >> > > it looks like we need to adjust that check to make sure we never >> > > even considering this situation (swapping with self). >> > > >> > > I also wonder if we want explicit check in live update with more >> > > meaningful assert :) >> > >> > Thanks for debugging this! The code above assumes that you're moving >> > an unscheduled instruction, which should never be above InsertPos for >> > top- down scheduling. If the instruction was in multiple ready Q's, >> > then you may attempt to schedule it multiple times. You can avoid >> this >> > by checking Su->isScheduled in your Strategy's pickNode. See >> > InstructionShuffler::pickNode for an example. I don't see an >> > equivalent check in ConvergingScheduler, but there probably should >> be. >> > >> > Another possibility to consider is something strange with >> DebugValues, >> > which I haven't tested much. >> > >> > I reproduced the same assert on arm and filed PR13719. I'm not sure >> > yet if it's exactly the same issue, but we can move the discussion >> there. >> > >> > We need a better assert in live update and probably the scheduler >> too. >> > Lang mentioned he may look at the issue today. Meanwhile, I hope my >> > suggestion above helps. >> > >> > -Andy >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Hi Sergei, Andy, 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. Sergei - thanks very much for the investigation. That should help me pin this down. Cheers, Lang. On Tue, Aug 28, 2012 at 2:33 PM, Sergei Larin <slarin at codeaurora.org> wrote:> Andy, Lang, > > Thanks for the suggestion. > > > I have spent more time with it today, and I do see some strange things in > liveness update. I am not at the actual cause yet, but here is what I got > so > far: > > I have the following live ranges when I start scheduling a region: > > R2 = [0B,48r:0)[352r,416r:5)... > R3 = [0B,48r:0)[368r,416r:5)... > R4 = [0B,32r:0)[384r,416r:4)... > R5 = [0B,32r:0)[400r,416r:4)... > > I schedule the following instruction (48B): > > 0B BB#0: derived from LLVM BB %entry > Live Ins: %R0 %R1 %D1 %D2 > 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 > 12B %vreg30<def> = LDriw <fi#-1>, 0; > mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 > 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] > IntRegs:%vreg31 > 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 > 28B %vreg106<def> = TFRI 16777216; > IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<< CurrentTop > 32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 > 48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Needs to move above 28B > 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] > IntRegs:%vreg37 > > In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc. > The MI move triggers liveness update, which first triggers SlotIndex > renumbering: > > *** Renumbered SlotIndexes 24-56 *** > > So my 48B becomes 56B, so after the update new live ranges look like this: > > R2 = [0B,56r:0)[352r,416r:5)... > R3 = [0B,56r:0)[368r,416r:5)... > R4 = [0B,48r:0)[384r,416r:4)... > R5 = [0B,48r:0)[400r,416r:4)... > > Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B (also > new > after renumbering. But happens to match another old one). > collectRanges for MI figures that it is moving a paired register, and > correctly(?) selects these two ranges to update for %R2:R3 > > [0B,56r:0)[368r,416r:5)... > [0B,56r:0)[352r,416r:5)... > > ___BUT____ after the update, my new ranges look like this: > > R2 = [0B,32r:0)[352r,416r:5)... > R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have become > 48r > R4 = [0B,48r:0)[384r,416r:4)... > R5 = [0B,48r:0)[400r,416r:4)... > .... > 0B BB#0: derived from LLVM BB %entry > Live Ins: %R0 %R1 %D1 %D2 > 8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 > 12B %vreg30<def> = LDriw <fi#-1>, 0; > mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 > 20B %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] > IntRegs:%vreg31 > 24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 > 32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 > <<<<<<<<<<<<<<<< Moved instruction > 40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106 > 48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 > 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] > IntRegs:%vreg37 > > This is not caught at this time, and only much later, when another > instruction is scheduled to __the same slot___ the old one "occupied" > (48B), > the discrepancy is caught by one of unrelated asserts... I think at that > time there are simply some stale aliases in liveness table. > > I'm going to continue with this tomorrow, but if this helps to identify a > lurking bug today, my day was worth it :) :) :) > > Sergei > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum. > > > > -----Original Message----- > > From: Andrew Trick [mailto:atrick at apple.com] > > Sent: Tuesday, August 28, 2012 3:47 PM > > To: Sergei Larin > > Cc: LLVM Developers Mailing List; Lang Hames > > Subject: Re: [LLVMdev] Assert in LiveInterval update > > > > On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at codeaurora.org> > > wrote: > > > > > > I've described that issue (see below) when you were out of town... I > > > think I am getting more context on it. Please take a look... > > > > > > So, in short, when the new MI scheduler performs move of an > > > instruction, it does something like this: > > > > > > // Move the instruction to its new location in the instruction > > stream. > > > MachineInstr *MI = SU->getInstr(); > > > > > > if (IsTopNode) { > > > assert(SU->isTopReady() && "node still has unscheduled > > dependencies"); > > > if (&*CurrentTop == MI) <<<<<<<<<<<<<<<<<< Here we > > make > > > sure that CurrentTop != MI. > > > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom); > > > else { > > > moveInstruction(MI, CurrentTop); > > > TopRPTracker.setPos(MI); > > > } > > > ... > > > > > > But in moveInstruction we use > > > > > > // Update the instruction stream. > > > BB->splice(InsertPos, BB, MI); > > > > > > And splice as far as I understand moves MI to the location right > > > __before__ InsertPos. Our previous check made sure that InsertPos !> > > MI, But I do hit a case, when MI == InsertPos--, and effectively > > tries > > > to swap instruction with itself... which make live update mechanism > > very unhappy. > > > > > > If I am missing something in intended logic, please explain, > > otherwise > > > it looks like we need to adjust that check to make sure we never even > > > considering this situation (swapping with self). > > > > > > I also wonder if we want explicit check in live update with more > > > meaningful assert :) > > > > Thanks for debugging this! The code above assumes that you're moving an > > unscheduled instruction, which should never be above InsertPos for top- > > down scheduling. If the instruction was in multiple ready Q's, then you > > may attempt to schedule it multiple times. You can avoid this by > > checking Su->isScheduled in your Strategy's pickNode. See > > InstructionShuffler::pickNode for an example. I don't see an equivalent > > check in ConvergingScheduler, but there probably should be. > > > > Another possibility to consider is something strange with DebugValues, > > which I haven't tested much. > > > > I reproduced the same assert on arm and filed PR13719. I'm not sure yet > > if it's exactly the same issue, but we can move the discussion there. > > > > We need a better assert in live update and probably the scheduler too. > > Lang mentioned he may look at the issue today. Meanwhile, I hope my > > suggestion above helps. > > > > -Andy > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120830/da0fec35/attachment.html>
Thanks Lang! I'll continue to dig dipper on my side, hopefully meet you
somewhere half way J
Sergei
---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
From: Lang Hames [mailto:lhames at gmail.com]
Sent: Thursday, August 30, 2012 3:17 PM
To: Sergei Larin
Cc: Andrew Trick; LLVM Developers Mailing List
Subject: Re: [LLVMdev] Assert in LiveInterval update
Hi Sergei, Andy,
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.
Sergei - thanks very much for the investigation. That should help me pin
this down.
Cheers,
Lang.
On Tue, Aug 28, 2012 at 2:33 PM, Sergei Larin <slarin at codeaurora.org>
wrote:
Andy, Lang,
Thanks for the suggestion.
I have spent more time with it today, and I do see some strange things in
liveness update. I am not at the actual cause yet, but here is what I got so
far:
I have the following live ranges when I start scheduling a region:
R2 = [0B,48r:0)[352r,416r:5)...
R3 = [0B,48r:0)[368r,416r:5)...
R4 = [0B,32r:0)[384r,416r:4)...
R5 = [0B,32r:0)[400r,416r:4)...
I schedule the following instruction (48B):
0B BB#0: derived from LLVM BB %entry
Live Ins: %R0 %R1 %D1 %D2
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
12B %vreg30<def> = LDriw <fi#-1>, 0;
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
20B %vreg31<def> = LDriw <fi#-2>, 0;
mem:LD4[FixedStack-2]
IntRegs:%vreg31
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
28B %vreg106<def> = TFRI 16777216;
IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<<
CurrentTop
32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Needs to move above 28B
96B %vreg37<def> = LDriw <fi#-8>, 0;
mem:LD4[FixedStack-8]
IntRegs:%vreg37
In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc.
The MI move triggers liveness update, which first triggers SlotIndex
renumbering:
*** Renumbered SlotIndexes 24-56 ***
So my 48B becomes 56B, so after the update new live ranges look like this:
R2 = [0B,56r:0)[352r,416r:5)...
R3 = [0B,56r:0)[368r,416r:5)...
R4 = [0B,48r:0)[384r,416r:4)...
R5 = [0B,48r:0)[400r,416r:4)...
Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B (also new
after renumbering. But happens to match another old one).
collectRanges for MI figures that it is moving a paired register, and
correctly(?) selects these two ranges to update for %R2:R3
[0B,56r:0)[368r,416r:5)...
[0B,56r:0)[352r,416r:5)...
___BUT____ after the update, my new ranges look like this:
R2 = [0B,32r:0)[352r,416r:5)...
R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have
become 48r
R4 = [0B,48r:0)[384r,416r:4)...
R5 = [0B,48r:0)[400r,416r:4)...
....
0B BB#0: derived from LLVM BB %entry
Live Ins: %R0 %R1 %D1 %D2
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
12B %vreg30<def> = LDriw <fi#-1>, 0;
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
20B %vreg31<def> = LDriw <fi#-2>, 0;
mem:LD4[FixedStack-2]
IntRegs:%vreg31
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
<<<<<<<<<<<<<<<< Moved
instruction
40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106
48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
96B %vreg37<def> = LDriw <fi#-8>, 0;
mem:LD4[FixedStack-8]
IntRegs:%vreg37
This is not caught at this time, and only much later, when another
instruction is scheduled to __the same slot___ the old one "occupied"
(48B),
the discrepancy is caught by one of unrelated asserts... I think at that
time there are simply some stale aliases in liveness table.
I'm going to continue with this tomorrow, but if this helps to identify a
lurking bug today, my day was worth it :) :) :)
Sergei
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.
> -----Original Message-----
> From: Andrew Trick [mailto:atrick at apple.com]
> Sent: Tuesday, August 28, 2012 3:47 PM
> To: Sergei Larin
> Cc: LLVM Developers Mailing List; Lang Hames
> Subject: Re: [LLVMdev] Assert in LiveInterval update
>
> On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at codeaurora.org>
> wrote:
> >
> > I've described that issue (see below) when you were out of
town... I
> > think I am getting more context on it. Please take a look...
> >
> > So, in short, when the new MI scheduler performs move of an
> > instruction, it does something like this:
> >
> > // Move the instruction to its new location in the instruction
> stream.
> > MachineInstr *MI = SU->getInstr();
> >
> > if (IsTopNode) {
> > assert(SU->isTopReady() && "node still has
unscheduled
> dependencies");
> > if (&*CurrentTop == MI)
<<<<<<<<<<<<<<<<<< Here
we
> make
> > sure that CurrentTop != MI.
> > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom);
> > else {
> > moveInstruction(MI, CurrentTop);
> > TopRPTracker.setPos(MI);
> > }
> > ...
> >
> > But in moveInstruction we use
> >
> > // Update the instruction stream.
> > BB->splice(InsertPos, BB, MI);
> >
> > And splice as far as I understand moves MI to the location right
> > __before__ InsertPos. Our previous check made sure that InsertPos
!> > MI, But I do hit a case, when MI == InsertPos--, and effectively
> tries
> > to swap instruction with itself... which make live update mechanism
> very unhappy.
> >
> > If I am missing something in intended logic, please explain,
> otherwise
> > it looks like we need to adjust that check to make sure we never even
> > considering this situation (swapping with self).
> >
> > I also wonder if we want explicit check in live update with more
> > meaningful assert :)
>
> Thanks for debugging this! The code above assumes that you're moving an
> unscheduled instruction, which should never be above InsertPos for top-
> down scheduling. If the instruction was in multiple ready Q's, then you
> may attempt to schedule it multiple times. You can avoid this by
> checking Su->isScheduled in your Strategy's pickNode. See
> InstructionShuffler::pickNode for an example. I don't see an equivalent
> check in ConvergingScheduler, but there probably should be.
>
> Another possibility to consider is something strange with DebugValues,
> which I haven't tested much.
>
> I reproduced the same assert on arm and filed PR13719. I'm not sure yet
> if it's exactly the same issue, but we can move the discussion there.
>
> We need a better assert in live update and probably the scheduler too.
> Lang mentioned he may look at the issue today. Meanwhile, I hope my
> suggestion above helps.
>
> -Andy
_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120830/1a1b641f/attachment.html>
Hi Lang,
Just one more quick question. in LiveIntervalAnalysis.cpp In
SlotIndex findLastUseBefore(unsigned Reg, SlotIndex OldIdx)
Did you really mean to use
for (MachineRegisterInfo::use_nodbg_iterator
UI = MRI.use_nodbg_begin(Reg),
UE = MRI.use_nodbg_end();
UI != UE; UI.skipInstruction()) {}
Aren't we currently dealing with units, not registers ?
SlotIndex LastUse = findLastUseBefore(LI->reg, OldIdx);
.and isn't MRI.use_nodbg_begin(Reg) expects a register, not a unit? .or did
I got it wrong again. Sorry to bug you on this.
Sergei
---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
From: Lang Hames [mailto:lhames at gmail.com]
Sent: Thursday, August 30, 2012 3:17 PM
To: Sergei Larin
Cc: Andrew Trick; LLVM Developers Mailing List
Subject: Re: [LLVMdev] Assert in LiveInterval update
Hi Sergei, Andy,
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.
Sergei - thanks very much for the investigation. That should help me pin
this down.
Cheers,
Lang.
On Tue, Aug 28, 2012 at 2:33 PM, Sergei Larin <slarin at codeaurora.org>
wrote:
Andy, Lang,
Thanks for the suggestion.
I have spent more time with it today, and I do see some strange things in
liveness update. I am not at the actual cause yet, but here is what I got so
far:
I have the following live ranges when I start scheduling a region:
R2 = [0B,48r:0)[352r,416r:5)...
R3 = [0B,48r:0)[368r,416r:5)...
R4 = [0B,32r:0)[384r,416r:4)...
R5 = [0B,32r:0)[400r,416r:4)...
I schedule the following instruction (48B):
0B BB#0: derived from LLVM BB %entry
Live Ins: %R0 %R1 %D1 %D2
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
12B %vreg30<def> = LDriw <fi#-1>, 0;
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
20B %vreg31<def> = LDriw <fi#-2>, 0;
mem:LD4[FixedStack-2]
IntRegs:%vreg31
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
28B %vreg106<def> = TFRI 16777216;
IntRegs:%vreg106<<<<<<<<<<<<<<<<<<<<<<<<<<<
CurrentTop
32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Needs to move above 28B
96B %vreg37<def> = LDriw <fi#-8>, 0;
mem:LD4[FixedStack-8]
IntRegs:%vreg37
In Hexagon %D1==%R0:R1 (double reg), %D2==%R2:R3 etc.
The MI move triggers liveness update, which first triggers SlotIndex
renumbering:
*** Renumbered SlotIndexes 24-56 ***
So my 48B becomes 56B, so after the update new live ranges look like this:
R2 = [0B,56r:0)[352r,416r:5)...
R3 = [0B,56r:0)[368r,416r:5)...
R4 = [0B,48r:0)[384r,416r:4)...
R5 = [0B,48r:0)[400r,416r:4)...
Then in LiveIntervals::handleMove OldIndex 56B and NewIndex is 32B (also new
after renumbering. But happens to match another old one).
collectRanges for MI figures that it is moving a paired register, and
correctly(?) selects these two ranges to update for %R2:R3
[0B,56r:0)[368r,416r:5)...
[0B,56r:0)[352r,416r:5)...
___BUT____ after the update, my new ranges look like this:
R2 = [0B,32r:0)[352r,416r:5)...
R3 = [0B,48r:0)[368r,416r:5)...<<<<< Bogus range, 56r should have
become 48r
R4 = [0B,48r:0)[384r,416r:4)...
R5 = [0B,48r:0)[400r,416r:4)...
....
0B BB#0: derived from LLVM BB %entry
Live Ins: %R0 %R1 %D1 %D2
8B %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27
12B %vreg30<def> = LDriw <fi#-1>, 0;
mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30
20B %vreg31<def> = LDriw <fi#-2>, 0;
mem:LD4[FixedStack-2]
IntRegs:%vreg31
24B %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26
32B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28
<<<<<<<<<<<<<<<< Moved
instruction
40B %vreg106<def> = TFRI 16777216; IntRegs:%vreg106
48B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29
96B %vreg37<def> = LDriw <fi#-8>, 0;
mem:LD4[FixedStack-8]
IntRegs:%vreg37
This is not caught at this time, and only much later, when another
instruction is scheduled to __the same slot___ the old one "occupied"
(48B),
the discrepancy is caught by one of unrelated asserts... I think at that
time there are simply some stale aliases in liveness table.
I'm going to continue with this tomorrow, but if this helps to identify a
lurking bug today, my day was worth it :) :) :)
Sergei
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum.
> -----Original Message-----
> From: Andrew Trick [mailto:atrick at apple.com]
> Sent: Tuesday, August 28, 2012 3:47 PM
> To: Sergei Larin
> Cc: LLVM Developers Mailing List; Lang Hames
> Subject: Re: [LLVMdev] Assert in LiveInterval update
>
> On Aug 28, 2012, at 8:18 AM, Sergei Larin <slarin at codeaurora.org>
> wrote:
> >
> > I've described that issue (see below) when you were out of
town... I
> > think I am getting more context on it. Please take a look...
> >
> > So, in short, when the new MI scheduler performs move of an
> > instruction, it does something like this:
> >
> > // Move the instruction to its new location in the instruction
> stream.
> > MachineInstr *MI = SU->getInstr();
> >
> > if (IsTopNode) {
> > assert(SU->isTopReady() && "node still has
unscheduled
> dependencies");
> > if (&*CurrentTop == MI)
<<<<<<<<<<<<<<<<<< Here
we
> make
> > sure that CurrentTop != MI.
> > CurrentTop = nextIfDebug(++CurrentTop, CurrentBottom);
> > else {
> > moveInstruction(MI, CurrentTop);
> > TopRPTracker.setPos(MI);
> > }
> > ...
> >
> > But in moveInstruction we use
> >
> > // Update the instruction stream.
> > BB->splice(InsertPos, BB, MI);
> >
> > And splice as far as I understand moves MI to the location right
> > __before__ InsertPos. Our previous check made sure that InsertPos
!> > MI, But I do hit a case, when MI == InsertPos--, and effectively
> tries
> > to swap instruction with itself... which make live update mechanism
> very unhappy.
> >
> > If I am missing something in intended logic, please explain,
> otherwise
> > it looks like we need to adjust that check to make sure we never even
> > considering this situation (swapping with self).
> >
> > I also wonder if we want explicit check in live update with more
> > meaningful assert :)
>
> Thanks for debugging this! The code above assumes that you're moving an
> unscheduled instruction, which should never be above InsertPos for top-
> down scheduling. If the instruction was in multiple ready Q's, then you
> may attempt to schedule it multiple times. You can avoid this by
> checking Su->isScheduled in your Strategy's pickNode. See
> InstructionShuffler::pickNode for an example. I don't see an equivalent
> check in ConvergingScheduler, but there probably should be.
>
> Another possibility to consider is something strange with DebugValues,
> which I haven't tested much.
>
> I reproduced the same assert on arm and filed PR13719. I'm not sure yet
> if it's exactly the same issue, but we can move the discussion there.
>
> We need a better assert in live update and probably the scheduler too.
> Lang mentioned he may look at the issue today. Meanwhile, I hope my
> suggestion above helps.
>
> -Andy
_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120831/5cb8c4d6/attachment.html>