[PATCH] D86395: InstCombine transform pattern "(~A & B) ^ A -> (A | B)" added

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 29 09:32:29 PDT 2020


spatel added a comment.

In D86395#2246092 <https://reviews.llvm.org/D86395#2246092>, @Jac1494 wrote:

> In D86395#2242393 <https://reviews.llvm.org/D86395#2242393>, @spatel wrote:
>
>> You are proposing to transform 4 commuted variations of a pattern, but your tests do not provide coverage for those variants. My test suggestion would hopefully provide that coverage. 
>> This might be clearer if you create the tests before this code patch and step through the transforms in a debugger. Look for InstCombinerImpl::SimplifyAssociativeOrCommutative() and getComplexity().
>
> @spatel ,Thank you so much for such informative reply. Now anything is pending in review which need to be update?

I don't see any changes to the tests from the earlier draft of the patch. That is, `test1` and `test2` are identical unless I'm not seeing correctly? Please open a separate review to add only the test file, so we can work through that independently of the code change.

You might consider adding tests to an existing file like llvm/test/Transforms/InstCombine/xor.ll instead of creating a new file for just ~4 tests. If my earlier comment about the tests did not make sense, please grep for "thwart" in this test directory to see examples.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86395/new/

https://reviews.llvm.org/D86395



More information about the llvm-commits mailing list