[PATCH] D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added
Jaydeep Chauhan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 09:26:50 PDT 2020
Jac1494 added a comment.
In D86395#2274541 <https://reviews.llvm.org/D86395#2274541>, @lebedev.ri wrote:
> In D86395#2274522 <https://reviews.llvm.org/D86395#2274522>, @Jac1494 wrote:
>
>> In D86395#2269436 <https://reviews.llvm.org/D86395#2269436>, @spatel wrote:
>>
>>> In D86395#2268634 <https://reviews.llvm.org/D86395#2268634>, @Jac1494 wrote:
>>>
>>>> test53 and test55 has been removed.
>>>
>>> We are going in circles. I'll give this 1 more try, and if it still doesn't make sense, then maybe another reviewer can better communicate what we expect for the tests:
>>>
>>> 1. There are 4 commutative patterns variations in the code.
>>> 2. Each one of those patterns should have a test (we should have at least 4 tests).
>>> 3. Each test corresponds to exactly one of the commutative patterns.
>>> 4. Each test should be in canonical form without this patch (the baseline CHECK lines should correspond exactly to the IR as written).
>>> 5. The 8 step check I provided earlier may be used to confirm that the tests provide the expected coverage for the code.
>>
>> @spatel, apologies for the confusion created due to multiple revisions.
>>
>> Let's consider the initial pattern and associated commutative version of this.
>>
>> initial pattern:-
>> (~A & B) ^ A -> (A | B)
>>
>> And 8 possible commutative version of this.
>> 1) (~A & B) ^ A -> (A | B)
>> 2) (~B & A) ^ B -> (A | B)
>> 3) (B & ~A) ^ A -> (A | B) --> identical to 1)
>> 4) A ^ (~A & B) -> (A | B) --> identical to 1)
>> 5) A ^ (B & ~A) -> (A | B) --> identical to 1)
>> 6) (A & ~B) ^ B -> (A | B) --> identical to 2)
>> 7) B ^ (~B & A) -> (A | B) --> identical to 2)
>> 8) B ^ (A & ~B) -> (A | B) --> identical to 2)
>>
>> As you may notice that all these patterns (IR) are reducing to 2 unique patterns (pattern 1 and pattern 2).
>> So pattern 1 and 2 are sufficient for the coverage of all commutative variations of the primary pattern.
>
> How so?
> If you only have a test for pattern 1 and pattern 2, how do you know that the commutative variants are handled?
>
>> Does this matches with you understandings ??
@lebedev.ri , I have manually tested and verified.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86395/new/
https://reviews.llvm.org/D86395
More information about the llvm-commits
mailing list