[PATCH] D139993: [LoopFusion] Sorting of undominated FusionCandidates crashes

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

looks fine, `collectFusionCandidates` does check `isControlFlowEquivalent` rather than dominance



================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:391-392
   /// into dominance order.
   /// If LHS dominates RHS and RHS post-dominates LHS, return true;
   /// IF RHS dominates LHS and LHS post-dominates RHS, return false;
   bool operator()(const FusionCandidate &LHS,
----------------
this comment needs to be updated


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:422
+    // needed.
+    bool bWrongOrder =
+        nonStrictlyPostDominate(LHSEntryBlock, RHSEntryBlock, DT, LHS.PDT);
----------------
aeubanks wrote:
> please use LLVM coding style, local variables are CamelCase without a prefix
nit: these local variables should still be capitalized


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:429-432
+      if (LNode->getLevel() > RNode->getLevel())
+        return true;
+      else
+        return false;
----------------
Narutoworld wrote:
> A more simplified version.
> 
> return (LNode->getLevel() > RNode->getLevel() );
no need for parens


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

https://reviews.llvm.org/D139993