[PATCH] D28694: Hexagon-specific loop idiom recognition

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:320
+  addExtensionsToPM(EP_LoopIdiom, MPM);
+
   MPM.add(createLoopIdiomPass());             // Recognize idioms like memset.
----------------
hfinkel wrote:
> mehdi_amini wrote:
> > This extension point could get its own separate patch.
> > 
> > It seems very targeted, if the only point is to have TargetSpecific loop idiom recognition, then why aren't some TTI implemented and used in LoopIdiomPass?
> I thought about suggesting this, but didn't because:
> 
>  a. It is not clear what from the existing loop idiom recognition pass could be reused
>  b. The target-specific logic might want access to other analyses, and that would be awkward if it is just a TTI callback.
> 
> What do you think?
> 
These are good points!

> It is not clear what from the existing loop idiom recognition pass could be reused

Main point here: the insertion point in the pipeline :)

>  The target-specific logic might want access to other analyses, and that would be awkward if it is just a TTI callback.

No sure what you mean by "awkward" with a TTI needing analysis, but that would definitely be a problem if the TTI would want an analysis that is not generic or not currently available in LoopIdiom.

The reason I think discussing this extension point in its own revision is because I think we usually have "generic" and "coarse grain" extension points (do we have another case of specific hook like that?).
I'm cautious about it because adding such hooks makes it harder to evolve/maintain the pipeline (ultimately what when we'll have a hook before/after every single pass?).


Repository:
  rL LLVM

https://reviews.llvm.org/D28694