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

[Jaydeep Chauhan via Phabricator via llvm-commits]

Jac1494 added a comment.

In D86395#2268209 <https://reviews.llvm.org/D86395#2268209>, @spatel wrote:

> In D86395#2268155 <https://reviews.llvm.org/D86395#2268155>, @Jac1494 wrote:
>
>> In D86395#2268096 <https://reviews.llvm.org/D86395#2268096>, @spatel wrote:
>>
>>> In D86395#2260871 <https://reviews.llvm.org/D86395#2260871>, @spatel wrote:
>>>
>>>> Try this experiment to see if your tests provide the coverage that we expect:
>>>>
>>>> 1. Replace the "m_c_XXX" matchers with the non-commute equivalent.
>>>> 2. Exactly 1 test should be modified.
>>>> 3. Add back only 1 of the commutative matchers.
>>>> 4. Exactly 2 tests should be modified.
>>>> 5. Invert the commutativity of the matchers (so the opposite matcher is now commutative).
>>>> 6. Exactly 2 tests should be modified.
>>>> 7. Make both matchers commutative.
>>>> 8. All 4 tests should be modified.
>>>
>>> Did you try this with the tests as shown in this draft of the patch?
>>
>> Yes, @spatel I have tried this cases with tests of this patch.
>
> Then I think something is wrong with your build environment. If I remove the commutative matchers, I get 0 test diffs.

@spatel it is also same for me. If I remove both the commutative matchers than I also get 0 test diffs.(change x_c_Xor with x_Xor and x_c_And with x_And)

> This makes sense given that test53 is still identical to test52 in terms of commutative matching (the only differences now are the value types).

Added test53 to cover vector test. I will update/remove the test53.


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

https://reviews.llvm.org/D86395