[MachineSink] Clear kill flags on operands outside loop

[critson]

Summary

If an instruction is sunk into a loop then any kill flags on
operands declared outside the loop must be cleared as these
will be live for all loop iterations.

Fixes #46827

[fhahn]

Could the test case by just a MIR test case that only runs MachineSink?

[MatzeB]

llvm/lib/CodeGen/MachineSink.cpp

1501–1509

What about this code that already exists, is it not working for this example? If so, can it be fixed?

[critson]

• Address reviewer comments
• Change to MIR test

[critson]

In D126754#3560211, @fhahn wrote:

Could the test case by just a MIR test case that only runs MachineSink?

Yes, I'll do this.

llvm/lib/CodeGen/MachineSink.cpp

1501–1509

RegsToClearKillFlags is only populated by SinkInstruction, the code path for SinkIntoCycle is different.
However I agree this should generally use the same methodology for clearing the flags, so I have changed the code to populate RegsToClearKillFlags as part of SinkIntoCycle instead of what I initially wrote.

[MatzeB]

LGTM

[MatzeB]

llvm/lib/CodeGen/MachineSink.cpp

1292

Change this to MO.readsReg() As we don't need to deal with things like undef uses or internal bundle reads.

[critson]

• readsReg() instead of isUse()

[critson]

• Rebase before commit

llvm/lib/CodeGen/MachineSink.cpp

LLVM_DEBUG(
         dbgs() << "CycleSink: Not Sinking, block too large to analyse.\n");
     return false;
   }
 
   LLVM_DEBUG(dbgs() << "CycleSink: Sinking instruction!\n");
   SinkBlock->splice(SinkBlock->SkipPHIsAndLabels(SinkBlock->begin()), Preheader,
                     I);
 
+  // Clear any kill flags on uses defined outside cycle
+  for (MachineOperand &MO : I.operands()) {
+    if (!MO.isReg() || !MO.isUse() || !MO.isKill())
+      continue;
+
+    for (const MachineInstr &MI : MRI->def_instructions(MO.getReg())) {
+      if (!Cycle->contains(MI.getParent())) {
+        MO.setIsKill(false);
+        break;
+      }
+    }
+  }
+
   // The instruction is moved from its basic block, so do not retain the
   // debug information.
   assert(!I.isDebugInstr() && "Should not sink debug inst");
   I.setDebugLoc(DebugLoc());
   return true;
 }
 
 /// Return true if a target defined block prologue instruction interferes
bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
   // After sinking, some debug users may not be dominated any more. If possible,
   // copy-propagate their operands. As it's expensive, don't do this if there's
   // no debuginfo in the program.
   if (MI.getMF()->getFunction().getSubprogram() && MI.isCopy())
     SalvageUnsunkDebugUsersOfCopy(MI, SuccToSinkTo);
 
   performSink(MI, *SuccToSinkTo, InsertPos, DbgUsersToSink);
 
   // Conservatively, clear any kill flags, since it's possible that they are no
   // longer correct.
   // Note that we have to clear the kill flags for any register this instruction
   // uses as we may sink over another instruction which currently kills the
   // used registers.
   for (MachineOperand &MO : MI.operands()) {
     if (MO.isReg() && MO.isUse())
       RegsToClearKillFlags.insert(MO.getReg()); // Remember to clear kill flags.
   }
 
   return true;
 }
 
 void MachineSinking::SalvageUnsunkDebugUsersOfCopy(
     MachineInstr &MI, MachineBasicBlock *TargetBlock) {
   assert(MI.isCopy());
   assert(MI.getOperand(1).isReg());

llvm/test/CodeGen/SystemZ/machinelicm-sunk-kill-flags.ll

+; RUN: llc -mtriple=s390x-linux-gnu -O3 -consthoist-gep -sink-insts-to-avoid-spills -verify-machineinstrs -o /dev/null %s
+; REQUIRES: asserts
+
+; Test that verification passes after MachineLICM.
+
+%0 = type <{ i32, [3 x i8] }>
+@b = external dso_local global [1 x %0], align 2
+
+define dso_local void @c() local_unnamed_addr #0 {
+bb:
+  br label %bb1
+
+bb1:                                              ; preds = %bb1, %bb
+  call void @llvm.memcpy.p0.p0.i64(ptr nonnull align 1 undef, ptr align 1 getelementptr inbounds ([1 x %0], ptr @b, i64 7, i64 0), i64 7, i1 true)
+  call void @llvm.memcpy.p0.p0.i64(ptr nonnull align 1 undef, ptr align 1 getelementptr inbounds ([1 x %0], ptr @b, i64 7, i64 0), i64 7, i1 true)
+  br label %bb1
+}
+
+; Function Attrs: argmemonly nofree nounwind willreturn
+declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg) #1
+
+attributes #0 = { "target-features"="+transactional-execution,+vector,+vector-enhancements-1,+vector-enhancements-2" }
+attributes #1 = { argmemonly nofree nounwind willreturn }