[PATCH] D82927: Intergerate Loop Peeling into Loop Fusion

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:731
+    // and a non-constant trip count.
+    unsigned TC0 = SE.getSmallConstantTripCount(FC0.L);
+    unsigned TC1 = SE.getSmallConstantTripCount(FC1.L);
----------------
Consider `const ` if not mutable


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:752
+          << "Difference is less than 0. FC1 (second loop) has more "
+             "iterations than the first one. Currently not supported.\n");
+    }
----------------
The full stop can usually be omitted.

http://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:779
+
+      FC0.PP.PeelCount = PeelCount;
+
----------------
No need to add too many new lines in this function. It actually harms readability (people can't see too many lines in a screen)


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:1347
     // Then modify the control flow and update DT and PDT.
-    SmallVector<DominatorTree::UpdateType, 8> TreeUpdates;
+    SmallVector<DominatorTree::UpdateType, 16> TreeUpdates;
 
----------------
Rationale for an increase? Large inline count can consume more stack space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82927