[PATCH] D71145: [InstCombine] Allow to limit the max number of iterations

[Jakub Kuderski via Phabricator via llvm-commits]

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

I ran some experiments to see how many iterations of InstCombine are necessary on real-world C++ applications. I paste the results below.
The numbers were collected by running InstCombine in a Full-LTO-like scenario: first I compiled all the code with `-O3` with *optimizations disabled*, then I linked it together (using gllvm), and gave it to opt with `-O3`.

| Name                        | Bitcode size [MB] | Max Iterations | Total InstCombine Executions | Total Iterations | Average Iterations Per Execution |
| --------------------------- | ----------------- | -------------- | ---------------------------- | ---------------- | -------------------------------- |
| sqlite3.bc                  | 3                 | 5              | 12685                        | 17907            | 1.41                             |
| wasm-dis.bc                 | 25                | 4              | 295002                       | 361302           | 1.22                             |
| wasm-metadce.bc             | 25                | 4              | 299757                       | 367117           | 1.22                             |
| wasm-reduce.bc              | 25                | 4              | 298667                       | 365802           | 1.22                             |
| wasm-as.bc                  | 25                | 4              | 296061                       | 362598           | 1.22                             |
| wasm-emscripten-finalize.bc | 25                | 4              | 296670                       | 363328           | 1.22                             |
| wasm-ctor-eval.bc           | 25                | 4              | 298052                       | 365195           | 1.23                             |
| wasm-shell.bc               | 25                | 4              | 299701                       | 367329           | 1.23                             |
| wasm-opt.bc                 | 26                | 4              | 310991                       | 381525           | 1.23                             |
| wasm2js.bc                  | 26                | 4              | 310483                       | 380472           | 1.23                             |
| asm2wasm.bc                 | 26                | 4              | 310153                       | 380159           | 1.23                             |
| llvm-dis.bc                 | 20                | 6              | 225957                       | 290632           | 1.29                             |
| llvm-as.bc                  | 25                | 8              | 478734                       | 619940           | 1.29                             |
| rippled.bc                  | 427               | 20             | 1130242                            | 1444180                | 1.28                             |
| opt.bc                      | 184               | 8              | 1640664                            | 2147381                | 1.31                             |
| lld.bc                      | 210               | 8              | 1902236                            | 2479378                | 1.30                             |
| clang-9.bc                  | 397               | 8              | 3505308                            | 4503068                | 1.28                             |  |
|

The maximum number of iterations required was 20, while `check-llvm` seems to require 4. This is in-line with what I see in the GPU code I'm working on. 
Based on this data, I think that 1000 should be a safe limit that can actually detect infinite loops in finite time.
@lebedev.ri

That being said, it'd be interesting to hand-craft an IR generator that forces instcombine to run arbitrary many iteration before reaching a fixpoint. Do you have an idea for a code pattern that will force such a behavior?



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:125
 
+static constexpr unsigned InstCombineDefaultMaxIterations = 1000;
+
----------------
lebedev.ri wrote:
> kuhar wrote:
> > lebedev.ri wrote:
> > > I think it should be
> > > ```
> > > static constexpr unsigned InstCombineDefaultMaxIterations = std::numeric_limits<unsigned>::max - 1;
> > > ```
> > > if the goal is to catch infinite loops while avoiding unintentional capping of iteration limit.
> > UINT_MAX is about 4*10^9. I don't think we will ever reach that number of iterations on any reasonable-sized input bitcode.
> Yes, that is kind-of the point, i'm not confident we won't reach `1000` in normal situations.
See my full answer below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71145