Arnold Schwaighofer
2012-Aug-30 20:20 UTC
[LLVMdev] MC Register mapping question (MCRegUnitIterator )
The code in collectRanges() does:
// Collect ranges for register units. These live ranges are computed on
// demand, so just skip any that haven't been computed yet.
if (TargetRegisterInfo::isPhysicalRegister(Reg)) {
for (MCRegUnitIterator Units(Reg, &TRI); Units.isValid(); ++Units)
if (LiveInterval *LI = LIS.getCachedRegUnit(*Units))
collectRanges(MO, LI, Entering, Internal, Exiting, OldIdx);
} else {
// Collect ranges for individual virtual registers.
collectRanges(MO, &LIS.getInterval(Reg),
Entering, Internal, Exiting, OldIdx);
}
As an experiment, you could replace getCachedRegUnit with getRegUnit
(the uncached version) to make verify we don't have a stale state in
the cache.
On Thu, Aug 30, 2012 at 3:01 PM, Sergei Larin <slarin at codeaurora.org>
wrote:> Arnold,
>
> It is not my code per say - this is what is done in
> LiveIntervalAnalysis.cpp collectRanges(), it wants to iterate over
"units"
> of D1 (whatever they are assumed to be), but right there it associates them
> with actual physical register live ranges (see the tread below), and it
does
> it wrongly. I was trying to pinpoint exactly to where the issue is, but
> there are too many interdependent links in this chain, so I am seeking
some
> domain knowledge :)
Jakob Stoklund Olesen
2012-Aug-30 21:13 UTC
[LLVMdev] MC Register mapping question (MCRegUnitIterator )
On Aug 30, 2012, at 1:20 PM, Arnold Schwaighofer <arnolds at codeaurora.org> wrote:> The code in collectRanges() does: > > // Collect ranges for register units. These live ranges are computed on > // demand, so just skip any that haven't been computed yet. > if (TargetRegisterInfo::isPhysicalRegister(Reg)) { > for (MCRegUnitIterator Units(Reg, &TRI); Units.isValid(); ++Units) > if (LiveInterval *LI = LIS.getCachedRegUnit(*Units)) > collectRanges(MO, LI, Entering, Internal, Exiting, OldIdx); > } else { > // Collect ranges for individual virtual registers. > collectRanges(MO, &LIS.getInterval(Reg), > Entering, Internal, Exiting, OldIdx); > } > > As an experiment, you could replace getCachedRegUnit with getRegUnit > (the uncached version) to make verify we don't have a stale state in > the cache.LIS.getRegUnit() will either return the cached live range or compute it from scratch. That won't help. It's calling getCachedRegUnit() here because there is no point in updating live ranges that haven't been computed yet. Sergei, we don't compute live ranges for physical registers any more. Only regunits. /jakob
Sergei Larin
2012-Aug-30 21:42 UTC
[LLVMdev] MC Register mapping question (MCRegUnitIterator )
Jacob, I think I see it now, but the problem remains - after renumbering slot indexes, live intervals have bogus data: Before update: 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)... 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 32B %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 <<<<<<<<<<<<<< 48B %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 <<<<<<<<<<<<<< 96B %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] IntRegs:%vreg37 After update is triggered, 32B becomes 48B, and 48B becomes 32B (a coincidental _swap_ of two indexes), and they trade locations (remember, I am scheduling instructions, and getting here while updating live ranges). Now it looks like this: *** Renumbered SlotIndexes 24-56 *** 0 8 %vreg27<def> = COPY %R1<kill>; IntRegs:%vreg27 12 %vreg30<def> = LDriw <fi#-1>, 0; mem:LD4[FixedStack-1](align=8) IntRegs:%vreg30 20 %vreg31<def> = LDriw <fi#-2>, 0; mem:LD4[FixedStack-2] IntRegs:%vreg31 24 %vreg26<def> = COPY %R0<kill>; IntRegs:%vreg26 32 %vreg28<def> = COPY %D1<kill>; DoubleRegs:%vreg28 <<<<<<<<<<<<<< 40 %vreg106<def> = TFRI 16777216; IntRegs:%vreg106 48 %vreg29<def> = COPY %D2<kill>; DoubleRegs:%vreg29 <<<<<<<<<<<<<< 56 64 80 96 %vreg37<def> = LDriw <fi#-8>, 0; mem:LD4[FixedStack-8] IntRegs:%vreg37 And from this point on live ranges look like this: R2 = [0B,56r:0)[352r,416r:5)... << ??? expect [0B,32r:0) here R3 = [0B,56r:0)[368r,416r:5)... << ??? expect [0B,32r:0) here R4 = [0B,48r:0)[384r,416r:4)... << fine R5 = [0B,48r:0)[400r,416r:4)... << fine As you can see from above, slot 56B is not even "defined"... Things go downhill from there... What is suspicious - the full swap of two indexes, but I am still unable to nail the exact location of the problem, hence the noise :) Sergei --- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation> -----Original Message----- > From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk] > Sent: Thursday, August 30, 2012 4:13 PM > To: Arnold Schwaighofer > Cc: Sergei Larin; LLVM Developers Mailing List > Subject: Re: [LLVMdev] MC Register mapping question (MCRegUnitIterator > ) > > > On Aug 30, 2012, at 1:20 PM, Arnold Schwaighofer > <arnolds at codeaurora.org> wrote: > > > The code in collectRanges() does: > > > > // Collect ranges for register units. These live ranges are > computed on > > // demand, so just skip any that haven't been computed yet. > > if (TargetRegisterInfo::isPhysicalRegister(Reg)) { > > for (MCRegUnitIterator Units(Reg, &TRI); Units.isValid(); > ++Units) > > if (LiveInterval *LI = LIS.getCachedRegUnit(*Units)) > > collectRanges(MO, LI, Entering, Internal, Exiting, > OldIdx); > > } else { > > // Collect ranges for individual virtual registers. > > collectRanges(MO, &LIS.getInterval(Reg), > > Entering, Internal, Exiting, OldIdx); > > } > > > > As an experiment, you could replace getCachedRegUnit with getRegUnit > > (the uncached version) to make verify we don't have a stale state in > > the cache. > > LIS.getRegUnit() will either return the cached live range or compute it > from scratch. That won't help. > > It's calling getCachedRegUnit() here because there is no point in > updating live ranges that haven't been computed yet. > > Sergei, we don't compute live ranges for physical registers any more. > Only regunits. > > /jakob