[LLVMdev] Help with definition of subregisters; spill, rematerialization and implicit uses

[Quentin Colombet]

Hi Mikael,

On Aug 25, 2014, at 3:33 AM, Mikael Holmén <mikael.holmen at ericsson.com> wrote:

> Hi Quentin,
> 
> On 08/23/14 01:43, Quentin Colombet wrote:
> > [...]
>> 
>> Alternative we could do the same thing as insertReload:
>> Walk all uses *and* defs and apply the rematerialization for each use or readsRegs.
>> Notice the different iterator for remat and for reload. They should be the same.
> 
> Excellent! Ok so now I did
> 
> @@ -932,15 +932,28 @@ void InlineSpiller::reMaterializeAll() {
>   // Try to remat before all uses of snippets.
>   bool anyRemat = false;
>   for (unsigned i = 0, e = RegsToSpill.size(); i != e; ++i) {
>     unsigned Reg = RegsToSpill[i];
>     LiveInterval &LI = LIS.getInterval(Reg);
> -    for (MachineRegisterInfo::use_bundle_nodbg_iterator
> -         RI = MRI.use_bundle_nodbg_begin(Reg), E = MRI.use_bundle_nodbg_end();
> -         RI != E; ) {
> -      MachineInstr *MI = &*(RI++);
> -      anyRemat |= reMaterializeFor(LI, MI);
> +
> +    for (MachineRegisterInfo::reg_bundle_iterator
> +           RegI = MRI.reg_bundle_begin(Reg), E = MRI.reg_bundle_end();
> +         RegI != E; ) {
> +      MachineInstr *MI = &*(RegI++);
> +
> +      // Debug values are not allowed to affect codegen.
> +      if (MI->isDebugValue()) {
> +        continue;
> +      }
> +
> +      // Analyze instruction.
> +      SmallVector<std::pair<MachineInstr*, unsigned>, 8> Ops;
> +      MIBundleOperands::VirtRegInfo RI =
> +        MIBundleOperands(MI).analyzeVirtReg(Reg, &Ops);
> +
> +      if (RI.Reads)
> +        anyRemat |= reMaterializeFor(LI, MI);
>     }
>   }
>   if (!anyRemat)
>     return;
> 
> So with this change InlineSpiller::reMaterializeAll is more similar to InlineSpiller::spillAroundUses, and now I'm not aware of any crashes anymore. :) My test case works, and I haven't seen any other tests suddenly failing.

Excellent, thanks for checking!

> 
> Is this a good change that should be done not only locally in my version?

Yes, this is a good change!
Please submit the actual patch to llvm-commit.

Thanks,
-Quentin

> 
> Thanks alot for your help!
> /Mikael
> 
>> 
>> I let you give it a try.
>> 
>> Thanks,
>> -Quentin
>>> 
>>> Thanks,
>>> Mikael
>> 
>> 
>>