[PATCH] D114339: [InstCombine] simplify (~A | B) ^ A --> ~( A & B)
Mehrnoosh Heidarpour via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 23 14:25:22 PST 2021
MehrHeidar marked an inline comment as done.
MehrHeidar added a comment.
In D114339#3146382 <https://reviews.llvm.org/D114339#3146382>, @spatel wrote:
> 1. Please commit the baseline tests, so we see diffs here.
> 2. I expect that will show that the commuted variations are not testing what they claim to be testing (you'll need to add instructions to prevent commuting).
> 3. Please add tests with extra uses - since we're creating 2 instructions, the fold should be good as long as any one of the intermediate values can be eliminated (has one use).
>
> Side note: @rampitec recently added a matcher that takes a binary opcode as an input. It probably doesn't make sense here, but it's something to think about if we're trying to make logic folds more regular/predictable ('DeMorganize' folds so we handle patterns where 'and' and 'or' operations are swapped).
Thanks for you suggestion.
I have modified the tests and committed them in D114436 <https://reviews.llvm.org/D114436>;
I did change the code as well;
Though it's still conservative in terms that both should have only use. Since, we were removing an instruction from a combination of 3, I thought if any of them have an extra use then the number of instructions after fold will remains 3.
Also, regarding your last suggestion, I am agree that it's a good idea to make these logic folds more predictable. I will start in this direction.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3619
+ // (~A | B) ^ A --> ~(A & B) -- There are 4 commuted variants.
+ if (match(&I, m_c_Xor(m_c_Or(m_Not(m_Value(A)), m_Value(B)), m_Deferred(A))))
+ return BinaryOperator::CreateNot(Builder.CreateAnd(A, B));
----------------
foad wrote:
> This probably needs some `m_OneUse`s (but I'm not sure how you decide exactly where to put them).
Thank you!
I added the `m_OneUse` as per you suggestion in new commit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114339/new/
https://reviews.llvm.org/D114339
More information about the llvm-commits
mailing list