[PATCH] D29897: [BypassSlowDivision] Use ValueTracking to simplify run-time checks

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

I still do not understand what we are trying to accomplish by writing x86-specific tests for this at all.

This is a 99% target-generic LLVM optimization.  The only target-specific part of this transformation is the bypass width.  Everything else is totally generic and has absolutely nothing to do with the target ISA.

I can understand having a few tests to check that the bypass widths are set as we expect.  But otherwise, it seems to me that we should test this exactly like we test every other target-independent pass in LLVM, and that's by checking LLVM IR and not x86 assembly.

I understand it can be frustrating to throw away work you've done.  In this case, however, the argument seems clear-cut.  We want to test using LLVM IR where possible, because that's the one language everyone in the LLVM community is assumed to know, and because it's the language that most closely reflects the transformations being made here.  Otherwise when these tests fail, someone who doesn't necessarily understand x86 assembly is going to have to read this code and understand whether the failure is meaningful.  This is a burden we should avoid where possible, and a burden that we do, by and large, avoid in LLVM as a matter of convention, if not policy.


https://reviews.llvm.org/D29897