[PATCH] D71250: [LV] Create new vector loop preheader so it contains vectorizer generated code only.

Wed Dec 11 02:21:57 PST 2019

rengolin added a comment.

In D71250#1778881 <https://reviews.llvm.org/D71250#1778881>, @ebrevnov wrote:

> These extra branches are very short leaved and optimized out by SimplifyCFG which happens just 3 passes after the vectorizer.

Sounds reasonable.

> Thus I don't believe it can cause any harm to the performance. I double check that LLVM's test-suite has no regressions.

Regression in performance or not showing any of those branches in asm output?

Unfortunately, the test-suite benchmark is not a complete set of relevant programs. :(

> This is definitely a subjective thing as there is no any formal metric to measure. IMHO having vectorizer generated code clearly separated from the rest makes IR more readable. It becomes easy to see whole structure of the vectorizer generated code.

Just to keep in mind, in the past, steps towards generating "better looking" IR have almost always lead us to poorer codegen. Usually, we consider this to be a weak reason, if one at all, and one that could have unintended consequences.

So, if your patch has other properties, it's best to focus on them.

> Having sad that it's surely not the main motivation for the change. The aim here is to simplify next patch (https://reviews.llvm.org/D71053) which benefits from being able to observe vectorizer generated code without doing extra book keeping.

It'd be interesting to know if this patch would also make other analises easier, too. That way, even if your following patch doesn't land, this one can benefit other parts of the vectoriser.

Can you think of some existing infrastructure in the vectoriser that would be easier with this change?

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2680
+  // No trip count check needed for predicated vectorization.
+  if (Cost->foldTailByMasking())
+    return;
----------------
ebrevnov wrote:
> rengolin wrote:
> > Are you sure this is semantically valid on all targets?
> That doesn't change existing semantics. It simply allows not to generate trivially dead code because otherwise there is an unconditional branch to vector loop.
Ah, my bad. I didn't see the condition invertion.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71250