[PATCH] D37076: [LICM] Allow sinking when foldable in loop

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:711
+                             const TargetTransformInfo *TTI) {
+  /// FIXME: for now we only check if the addressing mode defined by a GEP is
+  /// directly foldable into a load.
----------------
This isn't a very good FIXME because it doesn't explain what you might fix about it. Are there other things for which we might check?


================
Comment at: lib/Transforms/Scalar/LICM.cpp:715
+  if (GEP && isa<LoadInst>(UserI) && (I->getParent() == UserI->getParent())) {
+    SmallVector<const Value *, 4> Indices;
+    for (auto I = GEP->idx_begin(); I != GEP->idx_end(); ++I)
----------------
This can just be:

  return TTI->getUserCost(GEP) == TargetTransformInfo::TCC_Free;

(this code here to call getGEPCost seems to duplicate the implementation logic of getUserCost)

On that note, you might not even have to restrict this to GEPs used by Loads, but rather, you could allow all zero-cost instructions.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:774
+      // Check if the instruction is foldable with its user in the loop.
+      if (!ContainFoldableUsersInLoop && isFoldableInLoop(&I, UI, TTI)) {
+        ContainFoldableUsersInLoop = true;
----------------
The test for `!ContainFoldableUsersInLoop` limits us to looking for only one foldable user within the loop. Why?


https://reviews.llvm.org/D37076