[llvm-commits] [llvm] r118702 - in /llvm/trunk/lib/CodeGen: SplitKit.cpp SplitKit.h

[Evan Cheng]

On Nov 10, 2010, at 1:08 PM, Jakob Stoklund Olesen wrote:

> 
> On Nov 10, 2010, at 12:17 PM, Evan Cheng wrote:
> 
>> Nice. One question though:
>> 
>> 
>> On Nov 10, 2010, at 11:31 AM, Jakob Stoklund Olesen wrote:
>> 
>>> 
>>> +  // Make sure that openli is properly extended from Idx to the new copy.
>>> +  // FIXME: This shouldn't be necessary for remats.
>>> +  openli_.addSimpleRange(Idx, VNI->def, ParentVNI);
>>> 
>> 
>> Why the "fixme"? Are you restricting remat to instructions with no register operands?
> 
> No, I allow register operands as much as isTriviallyReMaterializable does, but I never extend live ranges of operands to do so. I require all operands to be live with the same value at the remat location. This check is implemented in LiveRangeEdit::allUsesAvailableAt().

Ah ok. We can potentially allow more remat by extending live ranges with more analysis. But that's later.

> 
> I think that isTriviallyReMaterializable is actually too conservative:
> 
>   // Only allow one virtual-register def, and that in the first operand.
>    if (MO.isDef() != (i == 0))
>      return false;
> 
>    // For the def, it should be the only def of that register.
>    if (MO.isDef() && (llvm::next(MRI.def_begin(Reg)) != MRI.def_end() ||
>                       MRI.isLiveIn(Reg)))
>      return false;
> 
>    // Don't allow any virtual-register uses. Rematting an instruction with
>    // virtual register uses would length the live ranges of the uses, which
>    // is not necessarily a good idea, certainly not "trivial".
>    if (MO.isUse())
>      return false;
> 
> Given that all used virtual registers have the same value, there is no need to restrict to only one use, or single-def.
> 
> I wonder if 'isTriviallyReMaterializable' means that the instruction can be rematted anywhere without further checks? If so, I should perhaps be using the basic TID::isRematerializable() flag instead?

Right. It's only meant for the existing brain dead remat.

> 
> 
> Anyway, the FIXME is an other matter entirely. When the splitter inserts copies, it needs to make sure that the COPY source register can reach the COPY instruction. When it remats the value instead of issuing a COPY, that shouldn't be necessary - in fact, the source register live range should be shortened since it is not needed for the remat.
> 
> The live range extension is still necessary because the splitter is computing the live range of the last register as the complement of all the other new registers. This will go away after I figure out how to truncate live ranges after rematerialization.

Ok, thanks.

Evan

> 
> /jakob
>