[PATCH] D21775: [InstCombine] Simplify and correct folding fcmps with the same children

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 11:55:53 PDT 2016


spatel added a comment.

This looks fantastic...if it's true. :)

So that's my problem: before we even get to this patch, we only have ~13 tests covering the 256 possible (16 * 16 predicates * 2 (and+or) / 2 for commutation) combinations of predicates.

Is there some clever way to get more coverage from those tests? If there's nothing obvious, we might as well just copy/paste the entire set into the existing regression test files. It's cheap. That way, we're certain that we're choosing the right predicate in all cases. The regression tests would then also serve as an unofficial assert that the enum values never change from their perfectly chosen values. IMO, an official assert and some loud comments in the code are warranted too.

Whether or not we agree that we should have the full regression test coverage, please note that there's a script to generate exact checks for regression tests:
$ utils/update_test_checks.py --help

This removes the need for CHECK-NOTs.

I've updated the old tests using that script in http://reviews.llvm.org/rL274046 and http://reviews.llvm.org/rL274047 .


http://reviews.llvm.org/D21775





More information about the llvm-commits mailing list