[PATCH] D21719: Unroll restructure

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi,

Please find a couple of question/remarks below:

> Fix potential overflow.


Is it possible to compose a test for it?

> Use BEInsns instead of constant 2.


Why do we need it? At least, we can make it `const` I think.

Thanks,
Michael


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:699
@@ +698,3 @@
+// and BEInsns.
+static bool isLoopSizeMeetUP(unsigned LoopSize,
+                             TargetTransformInfo::UnrollingPreferences &UP,
----------------
s/isLoopSizeMeetUP/doesLoopSizeMeetUP/

Also, what about something like `isLoopSizeLessThanThreshold`?

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:704-711
@@ -690,1 +703,10 @@
+  if (TT == Pragma)
+    return (uint64_t)(LoopSize - UP.BEInsns) * UP.Count +
+        UP.BEInsns <= PragmaUnrollThreshold;
+  else if (TT == Partial)
+    return (uint64_t)(LoopSize - UP.BEInsns) * UP.Count +
+        UP.BEInsns <= UP.PartialThreshold;
+  return (uint64_t)(LoopSize - UP.BEInsns) * UP.Count +
+      UP.BEInsns <= UP.Threshold;
+}
 // Returns true if unroll count was set explicitly.
----------------
Why can't we just pass `PragmaUnrollThreshold`/`UP.PartialThreshold`/`UP.Threshold instead` of `ThresholdType TT`?


Repository:
  rL LLVM

http://reviews.llvm.org/D21719