[PATCH] D77973: [VPlan] Move widening check for non-memory/non-calls to function (NFC).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6940
+bool VPRecipeBuilder::shouldWiden(Instruction *I, VFRange &Range) const {
+  assert(!isa<PHINode>(I) && "PHIs should have been handled earlier");
+  // Instruction should be widened, unless it is scalar after vectorization,
----------------
fhahn wrote:
> Ayal wrote:
> > Retain the !isa<Call, Load, Store> alongside !isa<PHI>?
> > To avoid using this method for such opcodes, as their widening decision is more complex.
> I've brought it back, but with the new code structure, it also required adding an early exit for calls, loads & stores, but I think that makes sense. What do you think?
Ahh, right. Perhaps better to early-exit here:


```
  // Calls and memory instructions that were not widened earlier should be scalarized.
  if (isa<CallInst>(Instr) || isa<LoadInst>(Instr) || isa<StoreInst>(Instr))
    return false;
  assert(!isa<PHINode>(I) && ...);
```


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6969
     case Instruction::BitCast:
     case Instruction::Br:
     case Instruction::FAdd:
----------------
fhahn wrote:
> Ayal wrote:
> > Branches should also find their way out of IsVectorizableOpcode and into an assert; no recipes are created for them.
> Will submit as a follow-up commit once this one lands.
Sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77973