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

[Nikolai Bozhenov via Phabricator via llvm-commits]

n.bozhenov marked 3 inline comments as done.
n.bozhenov added inline comments.


================
Comment at: test/CodeGen/X86/bypass-slow-division-fnv.ll:3
+; RUN: opt < %s -O3 -unroll-count=4 | llc -mattr=+idivq-to-divl | FileCheck %s
+; CHECK-NOT: divl
+
----------------
jlebar wrote:
> Like in the earlier discussion, can we just test this in LLVM IR?
> 
> Instead of testing loop unrolling, could we just run your testcase through opt -O3 -unroll-count=4 (maybe we only need unroll-count=2) and check that in?  Then here we'd just need to run codegenprepare and check the output of that.
> 
> We prefer for unit tests to test small units of code, instead of running the whole optimization pipeline.  Whole-pipeline tests are better suited for the test-suite.
> We prefer for unit tests to test small units of code, instead of running the whole optimization pipeline. Whole-pipeline tests are better suited for the test-suite. 

I believe that the main difference is that the test-suite is for execution tests and unit testing is for lit-tests, isn't it?

> Instead of testing loop unrolling, could we just run your testcase through opt -O3 -unroll-count=4 (maybe we only need unroll-count=2) and check that in? Then here we'd just need to run codegenprepare and check the output of that.

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).

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.


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.



https://reviews.llvm.org/D28200