[PATCH] D26521: [X86] Allow folding of reloads from stack slots when loading a subreg of the spilled reg

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Target/X86/X86InstrInfo.h:383
+  /// (or a subreg operand that feeds a store).
+  /// For stores, LoadMI is always null. For loads, LoadMI is non-null if
+  /// we're trying to fold an existing load instruction, and null if we are
----------------
zvi wrote:
> Can you please clarify the last sentence in the comment?
Basically, this is the same as the LoadMI parameter to InlineSpiller::foldMemoryOperand(). There it's described as "Load instruction to use instead of stack slot when non-null." but I'm not sure that's any clearer.

There are 3 cases InlineSpiller::foldMemoryOperand() supports:
1) Fold a store to a stack slot into a def.
2) Fold a load from a stack slot into a use.
3) Fold an existing load instruction into a use.

Cases (2) and (3) are distinguished by actually passing the load instruction we want to fold through the API.
I can try to write that down more concisely (but hopefully in a clearer way than it is right now).

Note that this doesn't actually use LoadMI, just verifies it's non-null, but, conceivably, it could be useful. Do you think it would be better if I just pass a bool here for now, and change the API if we eventually need the instruction?


================
Comment at: lib/Target/X86/X86InstrInfo.h:387
+  bool isSubregFoldable(MachineOperand &MO,
+                                MachineInstr *LoadMI) const override;
+
----------------
craig.topper wrote:
> Fix the formatting here to line up the arguments.
Whoops, will do, thanks.


https://reviews.llvm.org/D26521