[PATCH] D159136: [VPlan] Simplify HCFG construction of region blocks (NFC).

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:89
+      return nullptr;
+    // can assert SinglePred is the latch of its loop.
+    return SinglePred;
----------------
Ayal wrote:
> This comment should be replaced by an assert, or dropped.
> 
> Note (somewhere) that exit blocks are assumed to have a single predecessor.
Right, adjusted, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:175
+
+  bool RegionExists = Loop2Region.contains(LoopOfBB);
+  assert(RegionExists ^ isHeaderBB(BB, LoopOfBB) &&
----------------
Ayal wrote:
> Use lookup here as well instead of contains and []?
> The default value constructed in case of absence would be a nullptr, given that value type is VPRegionBlock*.
Updated, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:182
+    auto *RegionOfVPBB = new VPRegionBlock(
+        LoopOfBB->getHeader()->getName().str(), false /*isReplicator*/);
+    RegionOfVPBB->setParent(Loop2Region[LoopOfBB->getParentLoop()]);
----------------
Ayal wrote:
> Worth a comment, something like:
> // If BB's loop is nested inside another loop within VPlan's scope, the header of that enclosing loop was already visited and its region constructed and recorded in Loop2Region. That region is now set as the parent of VPBB's region. Otherwise it is set to null.
added, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159136