[PATCH] D45151: [LICM] Hoisting invariant.group loads

Piotr Padlewski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 13 16:00:37 PDT 2018


Prazek added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:1075
       // drop.  It is a compile time optimization, not required for correctness.
-      !isGuaranteedToExecute(I, DT, CurLoop, SafetyInfo))
+      !canKeepMetadata(I, DT, CurLoop, SafetyInfo))
     I.dropUnknownNonDebugMetadata();
----------------
sanjoy wrote:
> Not sure what this replacement adds -- do you plan to add more stuff to `canKeepMetadata` in the future?
It is a good way to document the code. This way I do not need to comment both calls to
isGuaranteedToExecute saying what is the logic behind it. It is also more future proof - if someone would like to
change the logic of canKeepMetadata, then all callers will be affected, which could not be the case if someone would only change one place.


Repository:
  rL LLVM

https://reviews.llvm.org/D45151





More information about the llvm-commits mailing list