[LoopUnrollAndJam] Allow unroll and jam loops forced by user.

[Whitney]

Summary

Allow unroll and jam loops forced by user.
LoopUnrollAndJamPass is still disabled by default in the NPM pipeline, and can be controlled by -enable-npm-unroll-and-jam.

[Meinersbur]

Why is this specific to the PowerPC-backend. Doesn't the LoopUnrollAndJamPass pick-up the transformation metadata already?

[Whitney]

In D87786#2277611, @Meinersbur wrote:

Why is this specific to the PowerPC-backend. Doesn't the LoopUnrollAndJamPass pick-up the transformation metadata already?

When -enable-npm-unroll-and-jam is false (default), LoopUnrollAndJamPass doesn't unroll and jam any loops, no matter UP.UnrollAndJam is true or not.
When -enable-npm-unroll-and-jam is true, LoopUnrollAndJamPass try to unroll and jam all loops with UP.UnrollAndJam set to true.
When invoking LoopUnrollAndJamPass directly, LoopUnrollAndJamPass doesn't unroll and jam any loops when UP.UnrollAndJam is false, even if the loops have the unroll and jam metadata.

UP.UnrollAndJam is only set to true on ARM, or by option allow-unroll-and-jam. We want to allow unroll and jam loops on PowerPC, but only if they are forced by user for now.

[dmgreen]

I guess in my mind unroll and jam was a lot like runtime loop unrolling, which has to be enabled per target.

If there is an explicit pragma though, that does sound like it should override the targets default decision. I think the logic in tryToUnrollAndJamLoop could be updated to handle that.

A test for that would be good to see too.

[dmgreen]

Nice. I was worried this would be a bit convoluted, but it's clean like this just setting UP.UnrollAndJam.

LGTM

[Meinersbur]

Thanks for the fix!

llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp

303–304

Seems the culprit was probably this line, making the backend's decision to not unroll a higher priority than the user's.

[Whitney]

llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp

303–304

I think now we get the priority right, 1. Option AllowUnrollAndJam 2. User pragma 3. backend's decision. :)

llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp

tryToUnrollAndJamLoop(Loop *L, DominatorTree &DT, LoopInfo *LI,
                       ScalarEvolution &SE, const TargetTransformInfo &TTI,
                       AssumptionCache &AC, DependenceInfo &DI,
                       OptimizationRemarkEmitter &ORE, int OptLevel) {
   TargetTransformInfo::UnrollingPreferences UP =
       gatherUnrollingPreferences(L, SE, TTI, nullptr, nullptr, OptLevel, None,
                                  None, None, None, None, None);
   TargetTransformInfo::PeelingPreferences PP =
       gatherPeelingPreferences(L, SE, TTI, None, None);
+
+  TransformationMode EnableMode = hasUnrollAndJamTransformation(L);
+  if (EnableMode & TM_Disable)
+    return LoopUnrollResult::Unmodified;
+  if (EnableMode & TM_ForcedByUser)
+    UP.UnrollAndJam = true;
+
   if (AllowUnrollAndJam.getNumOccurrences() > 0)
     UP.UnrollAndJam = AllowUnrollAndJam;
   if (UnrollAndJamThreshold.getNumOccurrences() > 0)
     UP.UnrollAndJamInnerLoopThreshold = UnrollAndJamThreshold;
   // Exit early if unrolling is disabled.
   if (!UP.UnrollAndJam || UP.UnrollAndJamInnerLoopThreshold == 0)
     return LoopUnrollResult::Unmodified;
 
   LLVM_DEBUG(dbgs() << "Loop Unroll and Jam: F["
                     << L->getHeader()->getParent()->getName() << "] Loop %"
                     << L->getHeader()->getName() << "\n");
 
-  TransformationMode EnableMode = hasUnrollAndJamTransformation(L);
-  if (EnableMode & TM_Disable)
-    return LoopUnrollResult::Unmodified;
-
   // A loop with any unroll pragma (enabling/disabling/count/etc) is left for
   // the unroller, so long as it does not explicitly have unroll_and_jam
   // metadata. This means #pragma nounroll will disable unroll and jam as well
   // as unrolling
   if (hasAnyUnrollPragma(L, "llvm.loop.unroll.") &&
       !hasAnyUnrollPragma(L, "llvm.loop.unroll_and_jam.")) {
     LLVM_DEBUG(dbgs() << "  Disabled due to pragma.\n");
     return LoopUnrollResult::Unmodified;

llvm/test/Transforms/LoopUnrollAndJam/pragma-explicit.ll

-; RUN: opt -loop-unroll-and-jam -allow-unroll-and-jam -unroll-runtime -unroll-partial-threshold=60 < %s -S | FileCheck %s
-; RUN: opt -aa-pipeline=tbaa,basic-aa -passes='loop-unroll-and-jam' -allow-unroll-and-jam -unroll-runtime -unroll-partial-threshold=60 < %s -S | FileCheck %s
+; RUN: opt -loop-unroll-and-jam -unroll-runtime -unroll-partial-threshold=60 < %s -S | FileCheck %s
+; RUN: opt -aa-pipeline=tbaa,basic-aa -passes='loop-unroll-and-jam' -unroll-runtime -unroll-partial-threshold=60 < %s -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; CHECK-LABEL: function
 ; The explicit metadata here should force this to be unroll and jammed 4 times (hence the %.pre60.3)
 ; CHECK: %.pre = phi i8 [ %.pre60.3, %for.cond1.for.cond.cleanup3_crit_edge.us ], [ %.pre.pre, %for.cond1.preheader.us.preheader.new ]
 ; CHECK: %indvars.iv.3 = phi i64 [ 0, %for.cond1.preheader.us ], [ %indvars.iv.next.3, %for.body4.us ]