[PATCH] D53355: Introducing VPPHINode

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Nikolay,

For what you describe, the patch looks good, ie. create a VPPHIInst. But it doesn't clearly show the benefits of doing so.

I can imagine that it would simplify reasoning and, as you say, avoid creating temporary instructions if not needed, but it would be good to have an example to base that claim.

Do you have the next patch somewhere? Would be good to put it in Phab, so we could look at it and make this review easier.

Thanks!
--renato



================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:142
 
+  VPInstruction *createPhiInstruction(Instruction *Inst = nullptr) {
+    VPInstruction *NewVPInst = new VPPHINode();
----------------
If you need Inst to be a PHI (or null), then maybe adding an assert would help, here.


Repository:
  rL LLVM

https://reviews.llvm.org/D53355