[PATCH] D106440: [IROutliner] Change Prioritization of Outlining to honor cost model

[Andrew Litteken via Phabricator via llvm-commits]

AndrewLitteken added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1357
+
+  Instruction *RealEndInstruction =
+      Region.Candidate->backInstruction()->getNextNonDebugInstruction();
----------------
paquette wrote:
> paquette wrote:
> > Comment?
> What happens if this is `nullptr`?
For now, since it's never at the end of a module, this should not be an issue as there is always a branch instruction following, but I will add an assertion.


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1382
+      return true;
+    return !this->InstructionClassifier.visit(ID.Inst);
+  });
----------------
paquette wrote:
> Do you need `this` here?
> 
> also you can probably just write
> 
> ```
> return (std::next(...)->Inst != ID.Inst...) || !InstructionClassifier.visit(ID.Inst);
> ```
I would rather keep it like this, the next few patches build on this and becomes more complex when the instruction could be a branch, and it isn't so easy to make it a one liner.


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

https://reviews.llvm.org/D106440