[PATCH] D64557: Add llvm.loop.licm.disable metadata

[Tim Corringham via Phabricator via llvm-commits]

timcorringham marked 2 inline comments as done.
timcorringham added a comment.

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.



================
Comment at: lib/Transforms/Scalar/LICM.cpp:335
+  // just exit.
+  if (hasDisableLICMTransformsHint(L)) {
+    return false;
----------------
Meinersbur wrote:
> 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.
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. 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64557/new/

https://reviews.llvm.org/D64557