I'm working on some enhancements to rematerialization that I hope to contribute. It's mostly working but I am running into one problem. It boils down to having spilled a register used by the remat candidate. I thought this is what getReMatImplicitUse is supposed to handle but it looks inconsistent to me. The comment says this: /// getReMatImplicitUse - If the remat definition MI has one (for now, we only /// allow one) virtual register operand, then its uses are implicitly using /// the register. Returns the virtual register. unsigned LiveIntervals::getReMatImplicitUse(const LiveInterval &li, MachineInstr *MI) const { But down in the code we have this: if (TargetRegisterInfo::isPhysicalRegister(Reg) && !allocatableRegs_[Reg]) continue; // FIXME: For now, only remat MI with at most one register operand. assert(!RegOp && "Can't rematerialize instruction with multiple register operand!"); RegOp = MO.getReg(); #ifndef NDEBUG break; #endif So if Reg is an allocatable physical register, it gets returned as an implicit use. This directly contradicts the comment when talks about virtual registers exclusively. So who's right here? -Dave
On Nov 16, 2011, at 9:15 AM, David Greene wrote:> I'm working on some enhancements to rematerialization that I hope to > contribute.What do you have in mind?> It's mostly working but I am running into one problem. It > boils down to having spilled a register used by the remat candidate. > > I thought this is what getReMatImplicitUse is supposed to handle but > it looks inconsistent to me. The comment says this: > > /// getReMatImplicitUse - If the remat definition MI has one (for now, we only > /// allow one) virtual register operand, then its uses are implicitly using > /// the register. Returns the virtual register. > unsigned LiveIntervals::getReMatImplicitUse(const LiveInterval &li, > MachineInstr *MI) const {I just deleted all of the spiller code in LiveIntervalAnalysis.cpp. The LiveIntervals::isReMaterializable() entry point is still around, but it is only getting called from CalcSpillWeights.cpp. If we can get rid of that use, it should be deleted as well.> #ifndef NDEBUG > break; > #endifScary...> So if Reg is an allocatable physical register, it gets returned as an > implicit use. This directly contradicts the comment when talks about > virtual registers exclusively. > > So who's right here?You want LiveRangeEdit::allUsesAvailableAt() which performs the same check today. The important point is that all registers read by the rematerialized instruction have the same value at the new site. That applies to both physical and virtual registers. There is some ambiguity about instructions reading reserved registers. For example, we want to allow instructions reading %rip to be moved, even though the register clearly has a different value at the new location. Reserved registers without any defs anywhere in the function are treated as ambient registers that can be read anywhere. These registers don't get a LiveInterval, and are skipped here: // Reserved registers are OK. if (MO.isUndef() || !lis.hasInterval(MO.getReg())) continue; Reserved registers with defs get LiveIntervals, and are value-checked just like virtual registers. Also note that unreserved, unallocatable registers exist. For example the ARM %cpsr status register. /jakob
Jakob Stoklund Olesen <stoklund at 2pi.dk> writes:> On Nov 16, 2011, at 9:15 AM, David Greene wrote: > >> I'm working on some enhancements to rematerialization that I hope to >> contribute. > > What do you have in mind?Rematting more types of loads.>> /// getReMatImplicitUse - If the remat definition MI has one (for now, we only >> /// allow one) virtual register operand, then its uses are implicitly using >> /// the register. Returns the virtual register. >> unsigned LiveIntervals::getReMatImplicitUse(const LiveInterval &li, >> MachineInstr *MI) const { > > I just deleted all of the spiller code in LiveIntervalAnalysis.cpp.It's still there in 3.0. :)>> #ifndef NDEBUG >> break; >> #endif > > Scary...Yeah, no kidding!>> So if Reg is an allocatable physical register, it gets returned as an >> implicit use. This directly contradicts the comment when talks about >> virtual registers exclusively. >> >> So who's right here? > > You want LiveRangeEdit::allUsesAvailableAt() which performs the same > check today.But not in 3.0, right?> The important point is that all registers read by the rematerialized > instruction have the same value at the new site. That applies to both > physical and virtual registers.Yes, absolutely. I actually do this check in my own code before I realized it was in getReMatImplicitUse. My check is more general. :)> There is some ambiguity about instructions reading reserved registers. > For example, we want to allow instructions reading %rip to be moved, > even though the register clearly has a different value at the new > location.Yes.> Reserved registers without any defs anywhere in the function are > treated as ambient registers that can be read anywhere. These > registers don't get a LiveInterval, and are skipped here: > > // Reserved registers are OK. > if (MO.isUndef() || !lis.hasInterval(MO.getReg())) > continue;Ok, that translates to getReMatImplicitUse pretty well.> Reserved registers with defs get LiveIntervals, and are value-checked > just like virtual registers.Yes.> Also note that unreserved, unallocatable registers exist. For example > the ARM %cpsr status register.Yes. So it seems the comment on getReMatImplicitUse in incorrect in that it looks at virtual and physical registers. The current (3.0) limitation of one register use is unfortunate but not a show-stopper for what I need to do. Thanks for your help, Jakob! -Dave