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

Nikolai Bozhenov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 5 06:50:11 PST 2017


n.bozhenov added a comment.

>> 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?

The reason is that `opt -codegenprapare` doesn't run BypassSlowDivision for X86. So, I have to run the whole X86 backend to check that the division is not bypassed.

I believe that the new version of the heuristic is much more robust than the previous one. However, a naive implementation didn't work initially. It turned out that hash values weren't recognized because of PHI nodes with undef incoming values generated by loop-unroll. I had to amend the patch to work around the problem.

Earlier I had problems with bitcast instructions. Only after running the whole pipeline test I found that sometimes long constants are hidden behind bitcast instructions.

Obviously, a manually created test would never contain such unexpected things.

Previously you suggested manually running the testcase through `opt -O3 ...` and checking that in. This wouldn't work either. Some future change in the middle-end will break the pattern recognition in codegenprepare but we never find it out because our codegen input stays the same.

So, I believe that it is worth having a full pipeline test to catch anything unexpected coming from the middle-end and breaking the pattern recognition.


https://reviews.llvm.org/D28200





More information about the llvm-commits mailing list