[PATCH] D111302: [VPlan] Add initial VPlan verification.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D111302#3117711 <https://reviews.llvm.org/D111302#3117711>, @Ayal wrote:

> Thanks for pushing this forward!
>
> Wondering about:
>
>> /// Note that currently there is still an exception for VPBlendRecipes.
>
>
>
>> // FIXME: At the moment, creating a mask might insert non-phi recipes before VPBlendRecipes.
>
> VPBlendRecipes are indeed exceptional (not only //currently//?), as they represent a 'select' rather than a 'phi'. Is the //FIXME //needed?

Good point, I removed the fixme in the committed version.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp:147
+        errs() << "Found phi-like recipe after non-phi recipe:";
+        RecipeI->dump();
+        errs() << "after\n";
----------------
akuegel wrote:
> The dump() method is not necessarily available, it is guarded by:
> #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> 
> You should guard the calls to it in the same way.
Thanks, this should already be fixed by acbefbf19f6c


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp:148
+      if (RecipeI->isPhi() && !isa<VPBlendRecipe>(&*RecipeI)) {
+        errs() << "Found phi-like recipe after non-phi recipes!\n";
+        return false;
----------------
Ayal wrote:
> worth spelling out the offenders?
Thanks, I updated the code to print the phi-like recipe and the previous (non-phi like)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111302