[llvm] [RegisterScavenging] Respect early-clobber when scavenging registers (PR #184814)

[Dominik Steenken via llvm-commits]

================
@@ -140,10 +140,14 @@ class RegScavenger {
   ///
   /// If \p AllowSpill is false, fail if a spill is required to make the
   /// register available, and return NoRegister.
+  /// If \p InspectNext is true, inspect the instruction at MBBI
+  /// for early-clobber def regs that need to be excluded from the set
+  /// of valid registers.
   Register scavengeRegisterBackwards(const TargetRegisterClass &RC,
                                      MachineBasicBlock::iterator To,
                                      bool RestoreAfter, int SPAdj,
-                                     bool AllowSpill = true);
+                                     bool AllowSpill = true,
+                                     bool InspectNext = false);
----------------
dominik-steenken wrote:

Ok, i'm going to try to lay out my (admittedly limited) understanding of this situation so we can hopefully agree on what i need to do here.

* Pre-RA, EC defs are handled via liveness intervals with sub-instruction granularity (SlotIndexes)
* Post-RA (here) these are no longer available
* `LiveRegUnits` and `findSurvivorBackwards` are using a simpler, instruction-granularity liveness concept, which can't really properly express EC defs
* This is because an EC def isn't really "live" going into an instruction, but nevertheless must be excluded from consideration

To me, this reads like we're using a data structure ( `LiveRegUnits`) , which encodes a concept (instruction-boundary liveness) that is slightly weaker than what we need in this situation (instruction-boundary availability for assignment = liveness + EC Def concerns). My initial patches handled this by essentially adding special case handling to `scavengeRegisterBackwards`.

If i understand you correctly, you are arguing that instead either  `LiveRegUnits`  (which would then via `RS.backward(I)` incorporate the relevant information automatically) or `findSurvivorBackwards` (and its call sites) should be updated to incorporate EC defs into their concept of liveness. That feels like a substantial change in semantics that would radiate out to every user of either. So, to clarify - is that what you are suggesting or did i misunderstand?

https://github.com/llvm/llvm-project/pull/184814