[PATCH] D26299: Improving the efficiency of the Loop Unswitching pass

Michael Zolotukhin via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 14 12:40:35 PST 2016


mzolotukhin added a comment.

Hi Abhilash,

Thanks for working on this, please find some comments below.

Michael



================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:603
+      while (currentLoop->contains(DomBB)) {
+        BranchInst *BInst = dyn_cast<BranchInst>(DomBB->getTerminator());
+
----------------
Nit: we have a `BI` variable, and here we create `BInst`. Can we use a more descriptive name for one of them to help distinguishing them?


================
Comment at: test/Transforms/LoopUnswitch/elseif-non-exponential-behavior.ll:2
+; REQUIRES: asserts
+; RUN: opt < %s -O3 -loop-unswitch -stats -disable-output 2>&1 | grep "2 loop-unswitch    - Number of branches unswitched" | count 1
+
----------------
Please don't run entire `-O3` pipeline in the test. It's slow and makes the test fragile (because passes in the pipeline change the input).

You can get IR just before loop-unswitching and use it as the test (and in the test run `opt -loop-unswitch` without `-O3`). To get full list of passes in O3 pipeline, you can run `opt -O3 -debug-pass=Arguments`.

Also, it's better to check what exactly what we get after loop-unswitching, not just look for a debug message. We use `FileCheck` tool for this, and annotate tests with special `CHECK` statements. The documentation is available here: http://llvm.org/docs/CommandGuide/FileCheck.html, and you can take a look at existing loop-unswitching tests if you need an example.


Repository:
  rL LLVM

https://reviews.llvm.org/D26299





More information about the llvm-commits mailing list