[PATCH] D95468: [LoopUnswitch] Add shortcut if unswitched path is a no-op.

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 31 12:16:27 PST 2021


jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/Scalar/LoopUnswitch.cpp:799
+    Info.PathIsNoop &=
+        L->getHeader()->getParent()->mustProgress() || hasMustProgress(L);
+
----------------
FWIW, any maximal trip count is fine, no need for a constant one.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnswitch.cpp:1094
+        return true;
+      }
     }
----------------
fhahn wrote:
> jdoerfert wrote:
> > Flip the order here. First the shorter case where we use `unswitchIfProfitable` then the more complex one. The way it is is harder to read.
> The main advantage of doing the 'no-op path' handling first is that it avoids full unswitching. If we do `unswitchIfProfitable` first, then we might unnecessarily do full unswitching. Functionally it shouldn't be a big deal, because loop-delete should just remove the loop again, but it skips a bit of unnecessary work.
I see, I didn't realize that, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95468



More information about the llvm-commits mailing list