# Details

# Diff Detail

- Repository
- rG LLVM Github Monorepo

### Event Timeline

I think we need to step back before we add a bunch of other matches.

For an expression with and/or/not, there should always be a Demorgan sibling where the and/or are swapped, right?

https://alive2.llvm.org/ce/z/ziSbZa

But we're transforming that into something that ends in 'xor', so it suggests we're missing some other 'xor' fold (and possibly another 'or' fold?).

Can we generalize the existing transforms or at least group them so we don't have a mess of incomplete logic folds?

What I realized we have couple deficiencies: we do not always reassociate as needed, in particular if a subexpression has multiple uses, and we do not demorgan to increase a number of operations even when that will lead to a further combining. I am not sure if that can be generalized without exploding compile time.

At the same time here I am trying at least to organize folds with a same LHS into a single block (and a single LHS match) which I have not done initially. For the demorganing I also have a subsequent patch (not yet ready, I am still working on tests) to add one more LHS. This LHS is `~(A | B) & C` and the demorganed form I am adding is `~A & ~B & C`. `matchDeMorgansLaws` does not convert latter to the former because we have a second use of the inner 'and'. I'd say in general it should not do it for multiply used values, but given the overall cost of the expressions here the benefit outweights it.

I see what you mean, yes it looks like yet another missing fold.

As far as I understand the difference between `visitOr` and `visintAnd` in this case: both are calling `InstCombinerImpl::SimplifyUsingDistributiveLaws`. The problem is:

For 'and' it creates `(~(a & c) | b) & (~(a & b))` -> `(a & b) ^ (~(a & c) | b)`. This is beneficial as it is one instruction less.

For 'or' it would create `(~(a | c) & b) | (~(a | b))` -> `~((a | b) ^ (~(a | c) & b))` using the same rules, same amount of operations, not beneficial.

The case `(a & b) ^ (~(a & c) | b)` -> `~(a & (b | c))` can use a separate match in the `visitXor` but given that extra 'not' operation which is created after swapping 'and' and 'or' operations in the source pattern I do not see how this can be generalized for a case where target is just one operation less and the same complexity if has to be inverted. In turn the pattern `(~(a | c) & b) | (~(a | b))` -> `~(a | (b & c))` saves a lot specifically due to its source complexity. I wish we have infrastructure though to express a pattern with swapped 'and' and 'or' in general.

Quite similarly we are not handling inverted case from D112276: https://alive2.llvm.org/ce/z/ywuzdS

This is done: `(c & ~(a | b)) | (b & ~(a | c))` --> `~a & (b ^ c)`, and this is not: `(c | ~(a & b)) & (b | ~(a & c))` --> `~(a & (b ^ c))`.

With all that said it looks like these patterns have to be extracted into a separate function and called for both 'or' and 'and'. But I don't think this shall preclude this NFC and the next D113141 to wait for it, these are mostly orthogonal to me.

I agree (just wanted to raise the idea of generalizing if possible) - this one looks like good cleanup / NFC. LGTM.