[PATCH] D113132: [InstCombine] Fuse checks for LHS (~(A | B) & C) | ... NFC.
Stanislav Mekhanoshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 5 10:30:11 PDT 2021
rampitec added a comment.
In D113132#3112041 <https://reviews.llvm.org/D113132#3112041>, @spatel wrote:
> 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.
> https://alive2.llvm.org/ce/z/ziSbZa
I see what you mean, yes it looks like yet another missing fold.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113132/new/
https://reviews.llvm.org/D113132
More information about the llvm-commits
mailing list