[PATCH] D57837: [LV] Prevent interleaving if computeMaxVF returned None.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7325
+  unsigned IC = 1;
+  unsigned UserIC = 1;
+  if (MaybeVF) {
----------------
fhahn wrote:
> Ayal wrote:
> > fhahn wrote:
> > > Ayal wrote:
> > > > fhahn wrote:
> > > > > Ayal wrote:
> > > > > > UserIC should always be set to Hints.getInterleave().
> > > > > Ah I thought you meant that we should ignore UserIC, in case computeMaxVF returned None, similar to the way we ignore UserVF in that case. Did I miss something?
> > > > UserIC should remain intact. Only IC must decide not to interleave if no (suitable) VPlan was built.
> > > The problem is we pick either UserIC or the computed IC a bit later on, so if we initialize UserIC unconditionally, we still hit exactly the problem in the test case. We could move the code to choose IC or UserIC into the conditional (together with the diagnostic messages)
> > Ah, right, my bad. How about setting `InterleaveLoop` below to false, with suitable debug/report message. That is actually the only change really needed: always calling CM.selectInterleaveCount() would in most cases return 1 immediately anyhow due to OptForSize (including the test case), and perform redundant computation only for the runtime ptr check && hasBranchDivergence case. But it is logical to avoid calling CM.selectInterleaveCount() below; or leave it to a separate refactoring of how VF&IC are set, possibly noting the potential waste in a TODO.
> Right, I moved the code to pick IC or UserIC and the diagnostics into the conditional. I also moved the VF == 1 diagnostic and the requirements check, as we might get different diagnostic ordering otherwise. What do you think?
Above thought was simply to add another

```
if (!MaybeVF && UserIC > 1) {
  // Tell the user interleaving was avoided up-front, despite being explicitly requested. 
  LLVM_DEBUG(dbgs() << "LV: ...\n");
  IntDiagMsg = std::make_pair(...);
  InterleaveLoop = false;
} else if // other cases for setting InterleaveLoop to false;
```

following the previous structure of setting
```
UserIC = Hints.getInterleave();
if (MaybeVF) {
  VF = *MaybeVF;
  IC = CM.selectInterleaveCount(OptForSize, VF.Width, VF.Cost);
}
```
Sounds right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57837