[PATCH] D21885: RegScavenging: Add scavengeRegisterBackwards()

[Matthias Braun via llvm-commits]

MatzeB added inline comments.

================
Comment at: include/llvm/CodeGen/RegisterScavenging.h:168-169
@@ -165,1 +167,4 @@
 
+  /// Make a register of the specific register class available from the current
+  /// position to \p To (inclusive). SPAdj is the stack adjustment due to call
+  /// frame, it's passed along to eliminateFrameIndex(). Returns the scavenged
----------------
t.p.northover wrote:
> Could you be explicit about what the range is here?
> 
> I'm guessing `To` is before `this->MBBI` and that "inclusive" only applies to both, so the range would be `[To, this->MBBI]` (i.e. neither `To` nor  `this->MBBI` may clobber the register).
Yes you guessed correctly; I'll change the wording to be clearer.


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:1169-1171
@@ +1168,5 @@
+
+  // The first definition writes the register, subsequent definitions are
+  // allowed but only if the instructions read+write the register (typical
+  // for two address code). Search for the first definition.
+  const TargetRegisterInfo *TRI = MRI.getTargetRegisterInfo();
----------------
t.p.northover wrote:
> I don't follow this 2-address exception. It looks like for something like
> 
>     1: %v0 = INST %v0, ...
>     [...]
>     2: %v0 = INST
>     [...]
>     3: INST %v0<kill>
> 
> we'd find DefMI is instruction 2 and look for a register that's free between 2 & 3, but then set %v0 to this register everywhere. What happens if that register is defined between 1 & 2?
> 
> It seems unlikely that the first Def will be an instruction like 1 (though not impossible `and w0, w0, #0` or something could exist), but then again the whole exception is pointless if it can't happen.
The idea here is that ideally we would only want 1 definition for each virtual register. However this would not work with two-address instructions as something like:

`
%v0 = XXX
%v1 = INST %v0<tied-0>
`
would give you no guarantees that v1 receives the same register as v0. So instead of just allowing a single definition I also allow additional definitions for instructions that read and write the register at the same time.

The example you presented would be invalid in this scheme as the first definition is not allowed: The first definition is not allowed to read the register while subsequent definitions are only allowed if the instruction reads the register at the same time. Also note that this is not a new requirement, this just documents the status-quo.

I can try to reword the comment, the code may be slightly confusing because the MachineRegisterInfo::def_iterator gives no ordering guarantees so I cannot enforce the rules/abort when they are broken.


Repository:
  rL LLVM

http://reviews.llvm.org/D21885