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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D36059#826240, @hfinkel wrote:

> In https://reviews.llvm.org/D36059#825518, @chandlerc wrote:
>
> > In https://reviews.llvm.org/D36059#825488, @mehdi_amini wrote:
> >
> > > It seems to me to conceptually belong to the backend. Why isn't this part of CodeGenPrepare (or injected by the target as part of its pre-ISel IR passes)?
> >
> >
> > It definitely is similar to CGP.
> >
> > The reason I didn't put it there after talking to folks (mostly Hal I think) was because I generally operate under the principle of "if it doesn't need to be in CGP, it should be separate" for maintenance, testing, etc. The usual case which necessitates transforms being in CGP is needing to participate in its iterative process, but that isn't true here. A common practical reason is because the logic is too small or isolated to really make sense as its own pass, but that doesn't seem to be true here as well.
>
>
> This is my opinion as well (as I think I expressed on IRC). CGP has accumulated a lot of different pieces of functionality because it makes sense for them to iterate (similar to why InstCombine has gotten that way). I see no reason for this to be part of that iterative scheme, and so it can be a separate pass (and, thus, it should be).


Sure, I agree with having a separate pass. My point was rather about the fact that it is added to the optimization pipeline instead of left up to the target "IR-lowering" passes.

@chandlerc's explanations make sense to me, but seeing it as part of `buildModuleOptimizationPipeline` is still a bit strange. I wonder if this shouldn't be split further to clearly identify the point where we start to do some "lowering" (if we agree to have such conceptual "stage" in the pipeline). A bit like I extracted the "function simplification" part of the pipeline (what runs in a CGSCC alternating with the inliner)


https://reviews.llvm.org/D36059