[PATCH] D98509: [LV] Calculate max feasible scalable VF.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D98509#2732628 <https://reviews.llvm.org/D98509#2732628>, @hubert.reinterpretcast wrote:

> I haven't reached the root cause yet, but it appears that the issue we're hitting can be avoided if the `MaxSafeVF` parameter is made pass-by-reference instead of pass-by-value.
> @sdesmalen, I hope the following is not too intrusive a change to your patch.

Hi @hubert.reinterpretcast, thanks for your investigation so far! I have gone through the code several times, but don't really understand why your suggestion fixes the issue. I've also built it with both GCC and Clang, with- and without sanitizers, and with valgrind.

Having said that, I'd be okay with applying your suggestion for the sake of re-landing the patch and not breaking a specific buildbot, but I would like to aim for a concrete date to remove the work-around. What do you think is sensible here?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5873
+  // Convenience function to return the minimum of two ElementCounts.
+  auto MinVF = [](const ElementCount &LHS, const ElementCount &RHS) {
+    assert((LHS.isScalable() == RHS.isScalable()) &&
----------------
@hubert.reinterpretcast Here MinVF returns a `const ElementCount &` instead of `ElementCount`, not sure if that could make a difference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98509