[PATCH] D88382: [VPlan] Turn VPReductionRecipe into a VPValue

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7959
       WidenRecipe->getParent()->insert(RedRecipe, WidenRecipe->getIterator());
       WidenRecipe->eraseFromParent();
 
----------------
fhahn wrote:
> Now that both VPWidenREcipe and VPRedcutionRecipe are VPValues, we have to update all users of WidenRecipe with RedRecipe.
I think these were happening in the other order, if I am understanding you correctly.  The VPWidenRecipe patch is added on top of this, and hopefully fixes up what you are referring to?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7967
                "Expected to replace a VPWidenSC");
         CompareRecipe->eraseFromParent();
       }
----------------
fhahn wrote:
> We need to update the users of VPWidenRecipe's VPValue or assert that there are none.
Hmm. This is the cmp in a min/max pattern. I am hoping that it would only have one user that we have removed above. At least, I can't seem to get it to vectorize when it has multiple uses. I'll make sure this is an assert in the VPWiden patch.


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

https://reviews.llvm.org/D88382