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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 08:33:22 PDT 2020


fhahn marked an inline comment as done.
fhahn 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,
----------------
Ayal wrote:
> 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) && ...);
> ```
Is there an advantage to have the early exit here? To me it seems to fit slightly better at the call site of shouldWiden, because there it is clearer what is happening (i.e. the code to handle calls, stores, loads is just above.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77973





More information about the llvm-commits mailing list