[PATCH] D41765: [CodeGen] Provide an advanced shrink-wrapping interface

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added inline comments.
Herald added a subscriber: hintonda.


================
Comment at: include/llvm/CodeGen/ShrinkWrapper.h:60
+  ///    BitVector values: 0     1  0
+  ///  This means that r0 is used in %bb.4 and the Stack and r0 are used in
+  ///  %bb.0.
----------------
This means that r0 is used in %bb.4 and the Stack and r1 are used in


================
Comment at: lib/CodeGen/DefaultShrinkWrapper.cpp:98
+
+bool DefaultShrinkWrapInfo::useOrDefCSROrFI(const MachineInstr &MI,
+                                            RegScavenger *RS) const {
----------------
Just to remind you that your previous commit (r322584) should be applied in this function. 


================
Comment at: lib/CodeGen/DefaultShrinkWrapper.cpp:178
+      HasUses.resize(1);
+      HasUses.set(0);
+    }
----------------
As we handle CSRs/FI all together in the current implementation, it will be good to mention that '0' implicitly means that. 


================
Comment at: lib/CodeGen/DefaultShrinkWrapper.cpp:393
+
+    DEBUG(dbgs() << "Final shrink wrap candidates:\nSave: " << Save->getNumber()
+                 << ' ' << Save->getName() << "\nRestore: "
----------------
I think Elt should be printed together here. 


================
Comment at: lib/CodeGen/DefaultShrinkWrapper.cpp:407
+      RestoreResult.resize(SWI.getNumElt());
+    RestoreResult.set(Elt);
+  }
----------------
Don't you need do increase NumCandidates here  ?


https://reviews.llvm.org/D41765