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

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

In https://reviews.llvm.org/D29897#686823, @n.bozhenov wrote:

> However, I don't think that dropping the tests is a good idea. Target-independent tests would check that the transformation itself is performed correctly. That's good but not enough. The existing tests check that the compiler indeed generates a good code for integer division


In LLVM we canonically would write two tests for this.  First, we would write an opt check for the transformation.  Then, we would write an llc test to check that divs are lowered correctly.

Since you already have IR inputs that exercise the pass, and you have a script to generate IR tests from them, I don't think this should actually involve a lot of work.  Indeed, it looks like Sanjay above already converted the tests here from llc to opt tests using a script.

I suspect there are already plenty of llc tests in the tree that check that x86 divs are lowered correctly.  But you certainly could look and add some if there are gaps.

> For instance, https://reviews.llvm.org/D28198 wouldn't be possible if there were no such detailed X86-specific llc tests.

I am not saying we should not have x86-specific tests.  That patch is for an x86-specific lowering, so it's totally appropriate that we have an llc test for it.

I'm also not saying that we should have no x86-specific tests for this pass.  Just that most of them should be generic.

Please understand why I am not willing to compromise on this.  I (and others in the LLVM community) may to be on the hook to maintain these tests.  I know almost nothing about the x86 ISA -- I work on GPUs.  I do not want to be on the hook to maintain large tests with tons of x86 assembly that I don't understand.  Moreover, if I am on the hook for this, I am not going to do a good job of it.  (For similar reasons, I wouldn't ask you to be on the hook to maintain tests with tons of GPU assembly.)  LLVM IR is the one language that everyone on this project is expected to understand.  So where possible (and I understand it's not always possible), we should write tests that use this language.

> Moreover, I'm not sure if it is at all possible to write correct opt tests for this transformation. The bypassed types are registered only if `TM.getOptLevel() >= CodeGenOpt::Default`. However, when running `opt -codegenprepare` the TM.getOptLevel() equals CodeGenOpt::None. Running `opt -O2 -codegenprepare`, obviously, is not an option because it runs all the optimizations in the middle-end.

One option would be to target nvptx -- unlike x86, we unconditionally register the bypass types.  (There's no GPU-specific code in these tests, since they're just opt tests.)

See e.g.  test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll.


https://reviews.llvm.org/D29897