[PATCH] D91481: [LoopUnroll] Discount uniform instructions in cost models

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:769
+    // iterations.  We should break the backedge, not unroll that.
+    if (LocalUniformSize + UP.BEInsns + 1 > LoopSize)
+      LocalUniformSize = 0;
----------------
reames wrote:
> Whitney wrote:
> > Do we need a test case for this case where the loop is not unrolled because the entire loop body is uniform?
> Any suggestions on how to write one?  This was mostly defensive programming.  Every loop structure I can think of is either a) infinite, or b) involves at least two instructions.  Given BEInst is configurable (why?)  the later isn't quite enough to fully disallow the case.
I was having problem picturing this scenario, that's why was thinking a test case would help. 
BEInsns is configureable due to it is compared with results from TTI?


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:785
+        if (SE.isLoopInvariant(SE.getSCEV(&I), &TheLoop))
+          Count++;
+      }
----------------
Given that `unsigned LoopSize = Metrics.NumInsts;`, and `NumInsts += TTI.getUserCost(&I, TargetTransformInfo::TCK_CodeSize);`, should we increment `Count` by `TTI.getUserCost(&I, TargetTransformInfo::TCK_CodeSize);` instead of 1?


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

https://reviews.llvm.org/D91481