[PATCH] D26872: Outliner: Add MIR-level outlining pass

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 14:39:38 PST 2017


MatzeB added a comment.

And a few more for the X86 part.



================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9764-9766
+  if (MI.modifiesRegister(X86::RIP, &RI) ||
+      MI.readsRegister(X86::RIP, &RI))
+    return false;
----------------
Heh, in theory every single x86 instruction modifies RIP. But I assume we don't model it like that in LLVM. In any way restricting this to reads(RIP) should be enough.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9776-9781
+  if (isLoadFromStackSlot(MI, Dummy) || isStoreToStackSlot(MI, Dummy))
+    return false;
+
+  if (isLoadFromStackSlotPostFE(MI, Dummy) ||
+      isStoreToStackSlotPostFE(MI, Dummy))
+    return false;
----------------
Are those tests necessary given that you already throw out operations with FrameIndex operands?


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9783
+
+  if (MI.isLabel())
+    return false;
----------------
You could use `MachineInstr::isPosition()` instead of checking for `isLabel()` and `isCFIInstruction()`


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9786
+
+  for (MachineOperand MOP : MI.operands())
+    if (MOP.isCPI() || MOP.isJTI() || MOP.isCFIIndex() || 
----------------
Better use `const MachineOperand &MOP` to avoid some copying.


================
Comment at: lib/Target/X86/X86InstrInfo.h:617
+  MachineBasicBlock::iterator
+  insertOutlinedCall(
+                     Module &M,
----------------
This linebreak seems unnecessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D26872





More information about the llvm-commits mailing list