[PATCH] D92285: [VPlan] Manage scalarized values using VPValues.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2935
 
+  State.Builder.SetInsertPoint(Builder.GetInsertPoint()->getParent(),
+                               Builder.GetInsertPoint());
----------------
a.elovikov wrote:
> nit: Should it be Builder.getInsertBlock() to avoid calling getParent on BasicBlock::end() iterator? It might be impossible to have the end iterator here, but the getInsertBlock version is generally safer, IMO.
Good point, adjusted!


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2956
   // the scalar map entry.
-  VectorLoopValueMap.setScalarValue(Instr, Instance, Cloned);
+  // VectorLoopValueMap.setScalarValue(Instr, Instance, Cloned);
+  State.set(Def, Instr, Cloned, Instance);
----------------
a.elovikov wrote:
> nit: I think commenting it out would be misleading for the reader. Can we just remove it?
Yes, the comment should be outdated now. I removed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92285