[PATCH] D122148: [SLP] Peak into loads when hitting the RecursionMaxDepth

Wed Mar 23 05:33:59 PDT 2022

ABataev added a comment.

In D122148#3401136 <https://reviews.llvm.org/D122148#3401136>, @vporpo wrote:

> Thank you for taking the time to measure the compilation time with an increased max-depth limit.
>
> I am not sure what the max-depth limit was designed to be used for, probably for testing gathers in lit test? But I don't think that it is a very good way of limiting compilation time. A better option would be to limit the total number of nodes in the graph, by simply counting the entries in `buildTree_rec()`. So I would assume that increasing the max-depth limit should be relatively safe. If we run into compile-time issues, we can introduce the `buildTree_rec()` entry counter. @ABataev @RKSimon any thoughts on this?

I don't think it is a good idea to increase/remove completely the limit for tree height. I tried to do something similar recently and found out lots of regressions. I think we can do this safely only after we have the implementation for partial subtree/subgraph vectorization, before that it is not profitable.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3998
+  if (Depth >= RecursionMaxDepth &&
+      !(S.MainOp && match(S.MainOp, m_ZExtOrSExtOrSelf(m_Load(m_Value()))))) {
     LLVM_DEBUG(dbgs() << "SLP: Gathering due to max recursion depth.\n");
----------------
It might be not profitable, e.g. if vector extension is not free, while scalar extension is free, and loads are not vectorizable. Also, what if you have an extension of a load and some other instructions, not loads/extractelements, etc.?

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

https://reviews.llvm.org/D122148