[PATCH] D86977: [IRSim][IROutliner] Limit to extracting regions that only require inputs

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/IROutliner.h:187
   /// can identify as having similarity, but are more complicated to outline.
-  struct InstructionAllowed
-      : public InstVisitor<InstructionAllowed, bool> {
+  struct InstructionAllowed : public InstVisitor<InstructionAllowed, bool> {
     InstructionAllowed() {}
----------------
Can these white space changes be in an NFC commit? 


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:179
+  // handle this case yet, and exit early.
+  if (Inserted || (GVNToConstantIt->second == CST))
+    return true;
----------------
can't this just be 

`return (Inserted || (GVNToConstantIt->second == CST));`

Avoid the branch and confusing code here. 


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:266
+  for (Value *Input : CurrentInputs) {
+    assert(Input && "Have a nullptr as an input");
+    assert(C.getGVN(Input).hasValue() &&
----------------
Does this need to be a SetVector of Value pointers? Can it be references instead? You can drop the assert on nullptr then. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86977/new/

https://reviews.llvm.org/D86977