[PATCH] D28200: [BypassSlowDivision] Do not bypass division of hash-like values

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

> That wouldn't make much sense. The recognized patterns are already checked with simpler tests I have put into bypass-slow-div-special-cases.ll. The purpose of this particular file is to make sure that the patterns we recognize are indeed the patterns produced by the middle-end at the maximal optimization level (I have removed the explicit unroll-count option).

It sounds like there's an implicit "on this particular testcase" at the end of this sentence.  "We are checking that we recognize the patterns produced by the middle-end at https://reviews.llvm.org/owners/package/3/ *on this particular testcase*."

> The problem this test tries to solve is that the PHI-heuristic in isHashLikeValue is quite fragile. For example, it could be broken if some loop optimization (unrolling/vectorization/whatever) would produce slightly different code with an additional instruction between XOR and REM. This test verifies that there are no such additional instructions and the code is correctly recognized as hash-like.

It seems to me that if you think the code is fragile as-is, we should try to make it less fragile, so that a test targeting one specific testcase isn't necessary.  For instance, we could increase the search depth some?  This would obviate the need for an uber-specific, full-pipeline testcase.  It would also keep us honest, in that we're not optimizing for one particular chunk of code.

> And since the set of middle-end optimizations and their parameters are target-specific, I would like to have this test X86-specific. It is not an automatically generated test, it has only one CHECK statement, and therefore it is not a problem to have it as an llc test.

If we we really wanted to run the full pipeline, I don't see why we couldn't simply run `opt -O3 | opt -codegenprepare`.  Can you think of a specific bug in this pass that would not be caught by this that would be caught by the lit test?


https://reviews.llvm.org/D28200