[PATCH] D82709: [MachineLICM] [PowerPC] hoisting rematerializable cheap instructions based on register pressure.

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

In D82709#2120751 <https://reviews.llvm.org/D82709#2120751>, @efriedma wrote:

> > when machine licm gets a cheap rematerializable instruction, it will also check the user inside the loop. if the user is also a loop invariant, do not hoist it blindly, instead it hoists the instruction after evaluating the register pressure
>
> Wouldn't it be simpler to just change the "When hoisting the last use of a value in the loop, that value no longer needs to be live in the loop" check?  We could check if the value is trivially rematerializable; if it is, we're not really helping register pressure by hoisting.


@efriedma Do you mean changing the register pressure estimate model in function `calcRegisterCost` and hoisting rematerializable instruction all the time in machine LICM but keeping its last use inside the loop, so RA can sink the rematerializable instruction down when necessary?

I made another fix like:

  diff --git a/llvm/lib/CodeGen/MachineLICM.cpp b/llvm/lib/CodeGen/MachineLICM.cpp
  index 98638b9..88e382c 100644
  --- a/llvm/lib/CodeGen/MachineLICM.cpp
  +++ b/llvm/lib/CodeGen/MachineLICM.cpp
  @@ -911,14 +911,18 @@ MachineLICMBase::calcRegisterCost(const MachineInstr *MI, bool ConsiderSeen,
   
       RegClassWeight W = TRI->getRegClassWeight(RC);
       int RCCost = 0;
  -    if (MO.isDef())
  +    // If MI is a def of a rematerializable instruction and it has only one use,
  +    // we can treat it as no cost because RA can sink MI right before its user.
  +    if (MO.isDef() && !(TII->isTriviallyReMaterializable(*MI, AA) && 
  +                        MRI->hasOneNonDBGUse(Reg)))
         RCCost = W.RegWeight;
       else {
         bool isKill = isOperandKill(MO, MRI);
  +      bool isRemat = TII->isTriviallyReMaterializable(*MRI->getVRegDef(Reg), AA); 
         if (isNew && !isKill && ConsiderUnseenAsDef)
           // Haven't seen this, it must be a livein.
           RCCost = W.RegWeight;
  -      else if (!isNew && isKill)
  +      else if (!isNew && isKill && !isRemat)
           RCCost = -W.RegWeight;
       }

The change is simpler than what I did in this patch(hoisting cheap instructions based on register pressure). And with above change, machine LICM will not hoist all pattern groups as expected. But unfortunately, RA can not sink down the hoisted rematerializable instruction(`LIS`), so there are still many spills.

I am thinking this may be not a good solution either as it ties two passes Machine LICM and RA together. It may also hard to maintain, if we change one pass we must also consider another pass.

I prefer to keep all the logic in Machine LICM, we check the register pressure for the rematerializable instruction directly and not hoist it when in high register pressure so the rematerializable instruction's last use in loop will not be hoisted automatically.

What do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82709/new/

https://reviews.llvm.org/D82709