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

[Gil Rapaport via Phabricator via llvm-commits]

gilr added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9045
+  State.ILV->widenPHIInstruction(cast<PHINode>(getUnderlyingValue()), RdxDesc,
+                                 StartV, State.UF, State.VF, this, State);
 }
----------------
No need to pass as arguments now that State itself is passed.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9207
+    State.set(this, VPhi, VPhi, Part);
+    State.set(getOperand(0), VPhi, Part);
   } else {
----------------
This is porting the resetVectorValue to State operations, which I think is fine by itself (I tend to agree with passing nullptr as the underlying value to VPPredInstPHI), but something else bothers me: It seems VPPredInstPHI doesn't take over the VPReplicate as the VPValue mapped to the predicated IR value during VPlan construction, which means that recipes constructed for users of predicated instructions use the ReplicateRecipe although it doesn't dominate them. We get away with this by overriding the IR's vector value, but for correct VP def-use we should have VPPredInstPHI take over as the registered VPValue in VPlan during construction. Does all that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92285