[PATCH] D43086: [PowerPC] Infrastructure work. Implement getting the opcode for a spill in one place.

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

Huge fan of refactorings :)

Some inline comments.

-eric



================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:364
+
+  for (unsigned i = 0; i < NUM_OPCODES_FOR_SPILL; i++) {
+    if (OpcodesForSpill[ArchIndex][i] == Opcode) {
----------------
Could probably just pass the subtarget down rather than have the index.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.h:112
 
+#define NUM_OPCODES_FOR_SPILL  15
+
----------------
Not a huge fan of this. Perhaps just have a local data structure inside the function?


================
Comment at: lib/Target/PowerPC/PPCRegisterInfo.h:95
 
-  bool requiresFrameIndexScavenging(const MachineFunction &MF) const override {
-    return true;
-  }
+  bool requiresFrameIndexScavenging(const MachineFunction &MF) const override;
 
----------------
This seems unrelated?


https://reviews.llvm.org/D43086