llvm dev - Aug 2012 - [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 :)

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

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