[PATCH] D30161: [LoopUnrolling] Peel loops with invariant backedge Phi input

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

This generally LGTM, except for some nits. But please wait for @reames as well.



================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:83
+    PHINode *Phi;
+    for (auto BI = Header->begin(); (Phi = dyn_cast<PHINode>(&*BI)); ++BI) {
+      Value *Input = Phi->getIncomingValueForBlock(BackEdge);
----------------
Why the parens around (Phi = ...)?


================
Comment at: test/Transforms/LoopUnroll/peel-loop-not-forced.ll:1
+; RUN: opt < %s -S -loop-unroll -indvars -loop-deletion -simplifycfg -instcombine | FileCheck %s
+
----------------
We generally prefer to test passes in isolation. Can you please make this a test for loop-unroll only?


https://reviews.llvm.org/D30161