[LLVMdev] Possible Remat Bug

[David A. Greene]

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