Add llvm.loop.licm.disable metadata

[timcorringham]

Summary

For some targets the LICM pass can result in sub-optimal code in some
cases where it would be better not to run the pass, but it isn't
always possible to suppress the transformations heuristically.

Where the front-end has insight into such cases it is beneficial
to attach loop metadata to disable the pass - this change adds the
llvm.loop.licm.disable metadata to enable that.

[fhahn]

For some targets the LICM pass can result in sub-optimal code in some
cases where it would be better not to run the pass, but it isn't
always possible to suppress the transformations heuristically.

Can you provide more details on what targets those are and what the problems there are? How would the frontend decide? If it is target-specific, a TargetTransformInfo hook might be more suitable.

[timcorringham]

The target in this case is AMDGPU, and the problem is that transformations can create excessive register pressure ( but I don't believe this is necessarily unique to that target). The front-end has additional information available which can sometimes be used to identify such cases.

This change will only affect code when the metadata is used, so shouldn't have any adverse effect.

[hliao]

please update the doc on transformation metadata

[timcorringham]

Update LangRef doc

Added the new metadata to LangRef.rst

Updating D64557: Add llvm.loop.licm.disable metadata

[arsenm]

Can you post a real example? I would expect LICM to be used as a canonicalization pass, and then operations would be sunk as appropriate

[timcorringham]

Unfortunately I don't believe I have an example that is suitable for publishing. The problem is basically that the code motion is generally a good thing, but it can increase live ranges significantly pushing register pressure up such that performance is degraded, but that impact isn't apparent at the point at which the transformation is performed. Disabling the pass for certain loops is a low-impact pragmatic (but not ideal) solution.

[hfinkel]

In D64557#1580627, @timcorringham wrote:

Unfortunately I don't believe I have an example that is suitable for publishing. The problem is basically that the code motion is generally a good thing, but it can increase live ranges significantly pushing register pressure up such that performance is degraded, but that impact isn't apparent at the point at which the transformation is performed. Disabling the pass for certain loops is a low-impact pragmatic (but not ideal) solution.

Is this a rematerialization problem? We do hoist things sometimes which turn into multiple instructions and then we can't rematerialize during RA.

[Meinersbur]

lib/Transforms/Scalar/LICM.cpp

335

There is a hasDisableAllTransformsHint (llvm.loop.disable_nonforced) metadata that was intended to disable all (high-level) loop transformations. The use case is to ensure that no pass changes the loop before a user-defined transformation is applied (e.g. a loop is not unrolled before #pragma clang loop interleave(enable) is applied).

The prefix llvm.loop. indicates that LICM is also such a loop transformation and therefore disabled by llvm.loop.disable_nonforced, but I don't think we want that.

Maybe use llvm.licm.disable?

Also, there are several schemes of enable/disable metadata. The other one to disable a transformations is e.g. !{"llvm.loop.vectorize.enable", i1 0}. I am not sure which one is preferable; we have the mix already anyway.

[timcorringham]

Changed the metadata name to llvm.licm.disable

[timcorringham]

Some cases can be undone by rematerialization, but not all, and it can involve a lot of effort which increases compile time. The metadata is a pragmatic approach which helps in some cases.

lib/Transforms/Scalar/LICM.cpp

335

I have changed the metadata name to llvm.licm.disable.

As you note there is a variety of schemes, and I can't see a reason to choose one over another, so I'll leave it as it is.

[Meinersbur]

@fhahn @arsenm @hfinkel Has the motivation question been sufficiently addressed?

test/Transforms/LICM/pragma-licm-disable.ll

6

By convention, the RUN lines are the first in the file, even before comments.

30–38

Could you simplify the test? Remove branching within the loop, at most one PHI, one load (and maybe a store so the load result is not unused).

[timcorringham]

Simplified the LIT test

[timcorringham]

Does anyone have any remaining concerns with this as it is now?

[Meinersbur]

Please wait a day before committing in case there are still questions about the motivation.

[fhahn]

IMO it's fine to have this solution to address your immediate needs. I still think it would be good if we could get a reproducer, so we can address this properly in LICM.

docs/LangRef.rst

.. code-block:: llvm
     ...
     !0 = !{!0, !1}
     !1 = !{!"llvm.loop.unroll.count", i32 4}
 
 '``llvm.loop.disable_nonforced``'
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 This metadata disables all optional loop transformations unless
-explicitly instructed using other transformation metdata such as
+explicitly instructed using other transformation metadata such as
 ``llvm.loop.unroll.enable``. That is, no heuristic will try to determine
 whether a transformation is profitable. The purpose is to avoid that the
 loop is transformed to a different loop before an explicitly requested
 (forced) transformation is applied. For instance, loop fusion can make
 other transformations impossible. Mandatory loop canonicalizations such
 as loop rotation are still applied.
 
 It is recommended to use this metadata in addition to any llvm.loop.*

 
 If loop versioning is necessary, this metadata defined the attributes
 the non-distributed fallback version will have. See
 :ref:`Transformation Metadata <transformation-metadata>` for details.
 
 '``llvm.loop.distribute.followup_all``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Thes attributes in this metdata is added to all followup loops of the
+The attributes in this metadata is added to all followup loops of the
 loop distribution pass. See
 :ref:`Transformation Metadata <transformation-metadata>` for details.
 
+'``llvm.licm.disable``' Metadata
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+This metadata indicates that loop-invariant code motion (LICM) should not be
+performed on this loop. The metadata has a single operand which is the string
+``llvm.licm.disable``. For example:
+
+.. code-block:: llvm
+
+   !0 = !{!"llvm.licm.disable"}
+
+Note that although it operates per loop it isn't given the llvm.loop prefix
+as it is not affected by the ``llvm.loop.disable_nonforced`` metadata.
+
 '``llvm.access.group``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 ``llvm.access.group`` metadata can be attached to any instruction that
 potentially accesses memory. It can point to a single distinct metadata
 node, which we call access group. This node represents all memory access
 instructions referring to it via ``llvm.access.group``. When an
 instruction belongs to multiple access groups, it can also point to a

include/llvm/Transforms/Utils/LoopUtils.h

 Optional<MDNode *>
 makeFollowupLoopID(MDNode *OrigLoopID, ArrayRef<StringRef> FollowupAttrs,
                    const char *InheritOptionsAttrsPrefix = "",
                    bool AlwaysNew = false);
 
 /// Look for the loop attribute that disables all transformation heuristic.
 bool hasDisableAllTransformsHint(const Loop *L);
 
+/// Look for the loop attribute that disables the LICM transformation heuristics.
+bool hasDisableLICMTransformsHint(const Loop *L);
+
 /// The mode sets how eager a transformation should be applied.
 enum TransformationMode {
   /// The pass can use heuristics to determine whether a transformation should
   /// be applied.
   TM_Unspecified,
 
   /// The transformation should be applied without considering a cost model.
   TM_Enable,

lib/Transforms/Scalar/LICM.cpp

 bool LoopInvariantCodeMotion::runOnLoop(
     Loop *L, AliasAnalysis *AA, LoopInfo *LI, DominatorTree *DT,
     TargetLibraryInfo *TLI, TargetTransformInfo *TTI, ScalarEvolution *SE,
     MemorySSA *MSSA, OptimizationRemarkEmitter *ORE, bool DeleteAST) {
   bool Changed = false;
 
   assert(L->isLCSSAForm(*DT) && "Loop is not in LCSSA form.");
 
+  // If this loop has metadata indicating that LICM is not to be performed then
+  // just exit.
+  if (hasDisableLICMTransformsHint(L)) {
+    return false;
+  }
+
   std::unique_ptr<AliasSetTracker> CurAST;
   std::unique_ptr<MemorySSAUpdater> MSSAU;
   bool NoOfMemAccTooLarge = false;
   unsigned LicmMssaOptCounter = 0;
 
   if (!MSSA) {
     LLVM_DEBUG(dbgs() << "LICM: Using Alias Set Tracker.\n");
     CurAST = collectAliasInfoForLoop(L, LI, AA);

lib/Transforms/Utils/LoopUtils.cpp

 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
 using namespace llvm;
 using namespace llvm::PatternMatch;
 
 #define DEBUG_TYPE "loop-utils"
 
 static const char *LLVMLoopDisableNonforced = "llvm.loop.disable_nonforced";
+static const char *LLVMLoopDisableLICM = "llvm.licm.disable";
 
 bool llvm::formDedicatedExitBlocks(Loop *L, DominatorTree *DT, LoopInfo *LI,
                                    MemorySSAUpdater *MSSAU,
                                    bool PreserveLCSSA) {
   bool Changed = false;
 
   // We re-use a vector for the in-loop predecesosrs.
   SmallVector<BasicBlock *, 4> InLoopPredecessors;
Optional<MDNode *> llvm::makeFollowupLoopID(
   FollowupLoopID->replaceOperandWith(0, FollowupLoopID);
   return FollowupLoopID;
 }
 
 bool llvm::hasDisableAllTransformsHint(const Loop *L) {
   return getBooleanLoopAttribute(L, LLVMLoopDisableNonforced);
 }
 
+bool llvm::hasDisableLICMTransformsHint(const Loop *L) {
+  return getBooleanLoopAttribute(L, LLVMLoopDisableLICM);
+}
+
 TransformationMode llvm::hasUnrollTransformation(Loop *L) {
   if (getBooleanLoopAttribute(L, "llvm.loop.unroll.disable"))
     return TM_SuppressedByUser;
 
   Optional<int> Count =
       getOptionalIntLoopAttribute(L, "llvm.loop.unroll.count");
   if (Count.hasValue())
     return Count.getValue() == 1 ? TM_SuppressedByUser : TM_ForcedByUser;

test/Transforms/LICM/pragma-licm-disable.ll

+; RUN: opt < %s -S -basicaa -licm | FileCheck %s
+
+; Check that the LICM pass does not operate on a loop which has the
+; llvm.licm.disable metadata.
+; CHECK-LABEL: @licm_disable
+; CHECK: entry:
+; CHECK-NOT: load
+; CHECK: do.body:
+; CHECK: load i64, i64* bitcast (i32** @in to i64*)
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+@in = internal unnamed_addr global i32* null, align 8
+@out = internal unnamed_addr global i32* null, align 8
+
+define void @licm_disable(i32 %N) {
+entry:
+  br label %do.body
+
+do.body:                                          ; preds = %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %do.body ]
+  %v1 = load i64, i64* bitcast (i32** @in to i64*), align 8
+  store i64 %v1, i64* bitcast (i32** @out to i64*), align 8
+  %inc = add nsw i32 %i.0, 1
+  %cmp = icmp slt i32 %inc, %N
+  br i1 %cmp, label %do.body, label %do.end, !llvm.loop !1
+
+do.end:                                           ; preds = %do.body
+  ret void
+}
+!1 = !{!1, !2}
+!2 = !{!"llvm.licm.disable"}