[InlineSpiller] simplify insertReload() NFC

[nickdesaulniers]

Summary

The repeated use of std::next() on a MachineBasicBlock::iterator was
clever, but we only need to reconstruct the iterator post creation of
the spill instruction.

This helps simplifying where we plan to place the spill, as discussed in
D77849.

From here, we can simplify the code a little by flipping the return code
of a helper.

[efriedma]

LGTM with one minor comment.

llvm/lib/CodeGen/InlineSpiller.cpp

934

Not sure renaming this helper is really useful; the old name is the property it's actually computing, which makes more sense than naming it after what you plan to do with the result.

Not a big deal either way.

[nickdesaulniers]

Thanks for the review!

llvm/lib/CodeGen/InlineSpiller.cpp

934

If we keep the old name, then we should not negate the return values as done in this diff. Instead, we'd negate the return value at the call site, which looks funny (just return what you want, no negation required).

bool IsRealSpill = !isFullUndefDef(*MI);

Thoughts? Also, doesn't matter to me. This way just has fewer negations in it.

[efriedma]

llvm/lib/CodeGen/InlineSpiller.cpp

934

If you might want to use this helper elsewhere, then making the helper positive makes more sense. But thinking about it a bit more, probably fine to leave your patch as-is, and we can reconsider if we ever find some other place that wants to use it.

[nickdesaulniers]

llvm/lib/CodeGen/InlineSpiller.cpp

934

Sure, I'm not opposed to changing it back later if needed. Thanks again for the review.

llvm/lib/CodeGen/InlineSpiller.cpp

void InlineSpiller::insertReload(unsigned NewVReg,
   LLVM_DEBUG(dumpMachineInstrRangeWithSlotIndex(MIS.begin(), MI, LIS, "reload",
                                                 NewVReg));
   ++NumReloads;
 }
 
 /// Check if \p Def fully defines a VReg with an undefined value.
 /// If that's the case, that means the value of VReg is actually
 /// not relevant.
-static bool isFullUndefDef(const MachineInstr &Def) {
+static bool isRealSpill(const MachineInstr &Def) {
   if (!Def.isImplicitDef())
-    return false;
+    return true;
   assert(Def.getNumOperands() == 1 &&
          "Implicit def with more than one definition");
   // We can say that the VReg defined by Def is undef, only if it is
   // fully defined by Def. Otherwise, some of the lanes may not be
   // undef and the value of the VReg matters.
-  return !Def.getOperand(0).getSubReg();
+  return Def.getOperand(0).getSubReg();
 }
 
 /// insertSpill - Insert a spill of NewVReg after MI.
 void InlineSpiller::insertSpill(unsigned NewVReg, bool isKill,
                                  MachineBasicBlock::iterator MI) {
   MachineBasicBlock &MBB = *MI->getParent();
 
   MachineInstrSpan MIS(MI, &MBB);
-  bool IsRealSpill = true;
-  if (isFullUndefDef(*MI)) {
+  MachineBasicBlock::iterator SpillBefore = std::next(MI);
+  bool IsRealSpill = isRealSpill(*MI);
+  if (IsRealSpill)
+    TII.storeRegToStackSlot(MBB, SpillBefore, NewVReg, isKill, StackSlot,
+                            MRI.getRegClass(NewVReg), &TRI);
+  else
     // Don't spill undef value.
     // Anything works for undef, in particular keeping the memory
     // uninitialized is a viable option and it saves code size and
     // run time.
-    BuildMI(MBB, std::next(MI), MI->getDebugLoc(), TII.get(TargetOpcode::KILL))
+    BuildMI(MBB, SpillBefore, MI->getDebugLoc(), TII.get(TargetOpcode::KILL))
         .addReg(NewVReg, getKillRegState(isKill));
-    IsRealSpill = false;
-  } else
-    TII.storeRegToStackSlot(MBB, std::next(MI), NewVReg, isKill, StackSlot,
-                            MRI.getRegClass(NewVReg), &TRI);
 
-  LIS.InsertMachineInstrRangeInMaps(std::next(MI), MIS.end());
+  MachineBasicBlock::iterator Spill = std::next(MI);
+  LIS.InsertMachineInstrRangeInMaps(Spill, MIS.end());
 
-  LLVM_DEBUG(dumpMachineInstrRangeWithSlotIndex(std::next(MI), MIS.end(), LIS,
-                                                "spill"));
+  LLVM_DEBUG(
+      dumpMachineInstrRangeWithSlotIndex(Spill, MIS.end(), LIS, "spill"));
   ++NumSpills;
   if (IsRealSpill)
-    HSpiller.addToMergeableSpills(*std::next(MI), StackSlot, Original);
+    HSpiller.addToMergeableSpills(*Spill, StackSlot, Original);
 }
 
 /// spillAroundUses - insert spill code around each use of Reg.
 void InlineSpiller::spillAroundUses(unsigned Reg) {
   LLVM_DEBUG(dbgs() << "spillAroundUses " << printReg(Reg) << '\n');
   LiveInterval &OldLI = LIS.getInterval(Reg);
 
   // Iterate over instructions using Reg.