[PATCH] D19553: Unroll pass restructure.

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 15:16:31 PDT 2016


mehdi_amini added a subscriber: mehdi_amini.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:710
@@ +709,3 @@
+    if (UP.AllowRemainder &&
+        (LoopSize - BEInsns) * UP.Count + BEInsns < UP.Threshold)
+      return true;
----------------
Gerolf wrote:
> Please move  (LoopSize - BEInsns) * UP.Count + BEInsns  to a function and comment properly. It will be invoked many times as the estimated size of the unrolled loop gets checked repeatedly.
I believe this hasn't been done.

And there is a discrepancy since some places are doing the arithmetic in 64 bits while others are doing it in 32 bits:

`UnrolledSize = (uint64_t)(LoopSize - BEInsns) * TripCount + BEInsns;` vs ` UnrolledSize = (LoopSize - BEInsns) * UP.Count + BEInsns;`

Coverity reports possible overflows:

>>>     CID 1356133:    (OVERFLOW_BEFORE_WIDEN)
>>>     Potentially overflowing expression "(LoopSize - BEInsns) * UP.Count" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "uint64_t" (64 bits, unsigned).



http://reviews.llvm.org/D19553





More information about the llvm-commits mailing list