[PATCH] D36258: Disable loop peeling during full unrolling pass.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D36258#830096, @davidxl wrote:

> Perhaps it is cleaner to
>
> 1. pass the information whether it is a full unroll to tryToUnroll
> 2. add a internal option FullUnrollAllowPeeling and make it off by default?  This will be similar to UnrollAllowPeeling flag, but takes precedence if it is full unroll.


While I'm somewhat fond of refactoring these interfaces to be less of a mess, would it be better in a follow-up patch? I would also hope it can tackle more than just peeling but encompass several of the numerous parameters.



================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:204
     UP.UpperBound = *UserUpperBound;
+  if (UserAllowPeeling.hasValue())
+    UP.AllowPeeling = *UserAllowPeeling;
----------------
davidxl wrote:
> Move this closer to line 188 above?
I think this location is more consistent with the surrounding code... I'm reluctant to deviate here from what seems like a very consistent pattern.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1289
+        /*Threshold*/ None, AllowPartialParam, RuntimeParam, UpperBoundParam,
+        /*AllowPeeling*/ None);
     Changed |= CurChanged;
----------------
Shouldn't this pass `AllowPeeling` since you added that variable above?


https://reviews.llvm.org/D36258