[MachineLICM] NFC - make safety of moving explicitly for IsLoopInvariantInst

[shchenz]

Summary

Currently, IsLoopInvariantInst returns true implicitly indicate that all the instruction's operands(virtual registers) are not in the same loop with the instruction itself. So it is always safe to move the instruction since all operands are defined outside of loop.

We want to extend the definition of loop invariant instruction. Even operands of instruction A defined in the same loop with A, A may still be a loop invariant instruction, for example, the operands are rematerializable instructions.

This patch makes safety of moving explicitly in IsLoopInvariantInst, so we can extend the definition of loop invariant instruction without considering the above implicitly indicating.

[efriedma]

llvm/lib/CodeGen/MachineLICM.cpp

219

Instead of returning the IsSafeToMove bit, maybe it would be simpler to add an input parameter "AllowRematOperands"?

[shchenz]

llvm/lib/CodeGen/MachineLICM.cpp

219

Make sense. Will do this after its child patch D82709's solution is determined.

[shchenz]

we decide to use another solution in https://reviews.llvm.org/D82709, so this NFC should not need any more.

llvm/lib/CodeGen/MachineLICM.cpp

private:
     void ProcessMI(MachineInstr *MI, BitVector &PhysRegDefs,
                    BitVector &PhysRegClobbers, SmallSet<int, 32> &StoredFIs,
                    SmallVectorImpl<CandidateInfo> &Candidates);
 
     void AddToLiveIns(unsigned Reg);
 
     bool IsLICMCandidate(MachineInstr &I);
 
-    bool IsLoopInvariantInst(MachineInstr &I);
+    bool IsLoopInvariantInst(MachineInstr &I, bool &IsSafeToMove);
 
     bool HasLoopPHIUse(const MachineInstr *MI) const;
 
     bool HasHighOperandLatency(MachineInstr &MI, unsigned DefIdx,
                                unsigned Reg) const;
 
     bool IsCheapInstruction(MachineInstr &MI) const;
 

 /// register spills caused by register pressure if there is little to no
 /// overhead moving instructions into loops.
 void MachineLICMBase::SinkIntoLoop() {
   MachineBasicBlock *Preheader = getCurPreheader();
   if (!Preheader)
     return;
 
   SmallVector<MachineInstr *, 8> Candidates;
+  bool IsSafeToMove = false;
   for (MachineBasicBlock::instr_iterator I = Preheader->instr_begin();
        I != Preheader->instr_end(); ++I) {
     // We need to ensure that we can safely move this instruction into the loop.
     // As such, it must not have side-effects, e.g. such as a call has.
-    if (IsLoopInvariantInst(*I) && !HasLoopPHIUse(&*I))
+    if (IsLoopInvariantInst(*I, IsSafeToMove) && IsSafeToMove &&
+        !HasLoopPHIUse(&*I))
       Candidates.push_back(&*I);
   }
 
   for (MachineInstr *I : Candidates) {
     const MachineOperand &MO = I->getOperand(0);
     if (!MO.isDef() || !MO.isReg() || !MO.getReg())
       continue;
     if (!MRI->hasOneDef(MO.getReg()))
bool MachineLICMBase::IsLICMCandidate(MachineInstr &I) {
   if (I.mayLoad() && !mayLoadFromGOTOrConstantPool(I) &&
       !IsGuaranteedToExecute(I.getParent()))
     return false;
 
   return true;
 }
 
 /// Returns true if the instruction is loop invariant.
-/// I.e., all virtual register operands are defined outside of the loop,
-/// physical registers aren't accessed explicitly, and there are no side
-/// effects that aren't captured by the operands or other flags.
-bool MachineLICMBase::IsLoopInvariantInst(MachineInstr &I) {
+/// I.e., all virtual register operands are either defined outside of the loop
+/// or rematerializable instructions, physical registers aren't accessed
+/// explicitly, and there are no side effects that aren't captured by the
+/// operands or other flags.
+/// If the instruction is loop invariant, we can indicate that the instruction
+/// is safe to be moved by setting IsSafeToMove. For example, if a instruction
+/// has an operand defined in same block, it is not safe to move this invariant
+/// instruction.
+bool MachineLICMBase::IsLoopInvariantInst(MachineInstr &I, bool &IsSafeToMove) {
+  IsSafeToMove = true;
   if (!IsLICMCandidate(I))
     return false;
 
   // The instruction is loop invariant if all of its operands are.
   for (const MachineOperand &MO : I.operands()) {
     if (!MO.isReg())
       continue;
 
if (Register::isPhysicalRegister(Reg)) {
         // which would clobber it.
         return false;
       }
     }
 
     if (!MO.isUse())
       continue;
 
-    assert(MRI->getVRegDef(Reg) &&
-           "Machine instr not mapped for this vreg?!");
+    MachineInstr* Def = MRI->getVRegDef(Reg);
+    assert(Def && "Machine instr not mapped for this vreg?!");
 
-    // If the loop contains the definition of an operand, then the instruction
-    // isn't loop invariant.
-    if (CurLoop->contains(MRI->getVRegDef(Reg)))
+    if (CurLoop->contains(Def)) {
+      // Can not hoist I to other block since it has operand in current block.
+      IsSafeToMove = false;
+      // It this operand is not a rematerializable instruction, I is a loop
+      // variant variable.
+      if (!TII->isTriviallyReMaterializable(*Def, AA))
         return false;
     }
+  }
 
   // If we got this far, the instruction is loop invariant!
   return true;
 }
 
 /// Return true if the specified instruction is used by a phi node and hoisting
 /// it could cause a copy to be inserted.
 bool MachineLICMBase::HasLoopPHIUse(const MachineInstr *MI) const {
MachineInstr *MachineLICMBase::ExtractHoistableLoad(MachineInstr *MI) {
   assert(NewMIs.size() == 2 &&
          "Unfolded a load into multiple instructions!");
   MachineBasicBlock *MBB = MI->getParent();
   MachineBasicBlock::iterator Pos = MI;
   MBB->insert(Pos, NewMIs[0]);
   MBB->insert(Pos, NewMIs[1]);
   // If unfolding produced a load that wasn't loop-invariant or profitable to
   // hoist, discard the new instructions and bail.
-  if (!IsLoopInvariantInst(*NewMIs[0]) || !IsProfitableToHoist(*NewMIs[0])) {
+  bool IsSafeToMove = false;
+  if (!IsLoopInvariantInst(*NewMIs[0], IsSafeToMove) || !IsSafeToMove ||
+      !IsProfitableToHoist(*NewMIs[0])) {
     NewMIs[0]->eraseFromParent();
     NewMIs[1]->eraseFromParent();
     return nullptr;
   }
 
   // Update register pressure for the unfolded instruction.
   UpdateRegPressure(NewMIs[1]);
 
bool MachineLICMBase::Hoist(MachineInstr *MI, MachineBasicBlock *Preheader) {
 
   // Disable the instruction hoisting due to block hotness
   if ((DisableHoistingToHotterBlocks == UseBFI::All ||
       (DisableHoistingToHotterBlocks == UseBFI::PGO && HasProfileData)) &&
       isTgtHotterThanSrc(SrcBlock, Preheader)) {
     ++NumNotHoistedDueToHotness;
     return false;
   }
+  bool IsSafeToMove = false;
   // First check whether we should hoist this instruction.
-  if (!IsLoopInvariantInst(*MI) || !IsProfitableToHoist(*MI)) {
+  if (!IsLoopInvariantInst(*MI, IsSafeToMove) || !IsSafeToMove ||
+      !IsProfitableToHoist(*MI)) {
     // If not, try unfolding a hoistable load.
     MI = ExtractHoistableLoad(MI);
     if (!MI) return false;
   }
 
   // If we have hoisted an instruction that may store, it can only be a constant
   // store.
   if (MI->mayStore())