[PATCH] D36059: [memops] Add a new pass to inject fast-path code for specific library function calls.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Transforms/Scalar/FastPathLibCalls.cpp:90
+  /// Checks whether a value is known non-zero at a particular location.
+  bool isKnownNonZero(Value *V, Instruction &I) {
+    // If value tracking knows enough, we're done.
----------------
davidxl wrote:
> Is it better to rely on instcombine and cfg simplification to get rid of the redundant zero guard which can be more general?
Sadly, they're *less* powerful than this. You'd need PRE or something similar to get it, powered by PredicateInfo and GVN. Maybe JumpThreading and LVI could get it?

All of these seem really heavy weight to run this late in the pipeline. =/


================
Comment at: lib/Transforms/Scalar/FastPathLibCalls.cpp:147
+
+  struct FastPathMemOpFramework {
+    BasicBlock *HeadBB;
----------------
davidxl wrote:
> Framework can mean something much different. How about just call it 'Info'?
It's more than the information. It represents the actual fastpath CFG framework (for lack of a better word) that has been injected into the function and needs to be populated with the particular memop's logic...

That said, I could totally add a comment. =] Would that help clarify enough?


https://reviews.llvm.org/D36059