[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 marked an inline comment as done.
chandlerc added a comment.

In https://reviews.llvm.org/D36059#825485, @sepavloff wrote:

> What is advantage of making this transformation a separate pass instead of doing it during selection?


We need to form a fairly complex control flow structure, including loops and indexing into arrays within the loop. We also need to do substantial cross-basic-block analysis. All of these are between hard and impossible within the DAG. We could do it at MI but that seems to provide few if any benefits and a lot of added complexity. A bunch of our "expand a loop here" or "inject a loop here" logic has been moved to IR to make it easier to cope with (atomics, etc).

> Some targets do not have support library and always expand mem intrinsics, the code making expansion is in `LowerMemIntrinsics.cpp`. Does it make sense to combine these implementations?

Possibly, but I'm not sure that it does. That lowering tries to produce a genuinely high-performance version, whereas this is trying to provide a *small* version with good performance for short lengths. I would expect this logic to only make sense when we're going to call a dedicated routine to handle most of the cases for code-size reasons.

Anyways, if we discover some useful components or pieces to share, we of course should, but I feel like the intent of the two passes is fairly distinct and useful to keep separate even if they share infrastructure.



================
Comment at: lib/Transforms/Scalar/FastPathLibCalls.cpp:443
+
+  DT.verifyDomTree();
+
----------------
sepavloff wrote:
> Probably this function should be called only when `VerifyDomInfo` is `true` to reduce compile time?
Sorry, this was just a debugging line. I added a simple pass-based verification to the tests, I'll remove this entirely.


https://reviews.llvm.org/D36059