[PATCH] D114586: [VPlan] Verify plan entry and exit blocks, set correct exit block.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9562
          "entry block must be set to a VPRegionBlock having a non-empty entry "
          "VPBasicBlock");
   RecipeBuilder.fixHeaderPhis();
----------------
Ayal wrote:
> Wonder if the above assert should also be taken care of by VPlan verify instead?
Could do, the only missing piece would be the non-empty part. I think that makes sense here, but might be overly restrictive as general requirement?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9633
 
+  cast<VPRegionBlock>(Plan->getEntry())->setExit(VPBB);
   // Now that sink-after is done, move induction recipes for optimized truncates
----------------
Ayal wrote:
> Another way of fixing this would be to have split-blocks update Exit, when it updates VPBB?
Yes, I can change it, but I think that be a bit more work.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp:169
+    return false;
+  }
+  const VPBasicBlock *Exit = dyn_cast<VPBasicBlock>(TopRegion->getExit());
----------------
Ayal wrote:
> The Entry and Exit blocks of any region in Plan must have no predecessors and successors, respectively, by definition; not only the top-most region, e.g., including internal replicating regions. Worth extending to check all regions in Plan? Entry and Exit blocks may themselves be regions, in general, so asserting that they be basic blocks should be restricted to the top-most region.
Done, I updated the code to verify empty predecessors/successor for all region entries/exits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114586