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

[Nikolai Bozhenov via Phabricator via llvm-commits]

n.bozhenov added a comment.

That's indeed somewhat frustrating... As I understand, now you all believe it's better to drop all the tests from test/CodeGen/X86/bypass-slow-division-32.ll and test/CodeGen/X86/bypass-slow-division-64.ll and replace them with a few target-independent tests.

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, which is the only purpose of the transformation, and this cannot be checked with opt tests. For instance, https://reviews.llvm.org/D28198 wouldn't be possible if there were no such detailed X86-specific llc tests. Only because of the tests, the inefficiency introduced with https://reviews.llvm.org/D28196 was detected immediately and fixed right away.

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.


https://reviews.llvm.org/D29897