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

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D21775#469307, @timshen wrote:

> > If there's nothing obvious, we might as well just copy/paste the entire set into the existing regression test files.
>
>
> I guess I missed a bit of context. Copy from where? By saying regression test files do you mean test/Transforms/InstCombine/and-fcmp.ll and test/Transforms/InstCombine/or-fcmp.ll?


Yes, the files under the 'test' dir are the regression tests:
http://llvm.org/docs/TestingGuide.html#llvm-testing-infrastructure-organization

Sorry, 'copy/paste' wasn't the best phrase. I just meant we should stamp out all N of the logical combinations of predicates. Right now, we don't really know what the existing behavior for each combination looks like.

> Besides, auto-generate the CHECKs (assertions?) won't help with the LLVM code coverage, will it? If not, should we generate all 512 (regardless of the commutations) cases, since it's not that many? But then we have to test the generator... :P.


I don't understand what you mean by 'test the generator'?

Here's how I would proceed:

1. Create all N tests in the existing test files. I don't think there's much value in the commuted variants, so this would actually be N = 16*17 = 272 I think.
2. Run the script to generate the CHECK lines.
3. Commit these test changes to trunk.
4. Now, we know what predicates the existing code produces (hopefully there are no existing bugs).
5. Apply your code patch.
6. Regenerate the CHECK lines using the script.
7. Update the patch here with those diffs, so we can see any functional differences from your change in logic.


http://reviews.llvm.org/D21775