This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] (~a & ~b & c) | (~a & ~c & b) --> (b ^ c) & ~a
AbandonedPublic

Authored by rampitec on Dec 23 2021, 11:51 AM.

Details

Summary

and swapped (~a | ~b | c) & (~a | ~c | b) --> ~((b ^ c) & a).
This is the extension of handled case (~(a | b) & c) | (~(a | c) & b)
--> (b ^ c) & ~a with expression not being demorganed because
of mutiple uses of ~a and/or ~b.

The rest of the cases with the same LHS are guarded to use only
the original demorganed form for now.

Diff Detail

Unit TestsFailed

Event Timeline

rampitec created this revision.Dec 23 2021, 11:51 AM
rampitec requested review of this revision.Dec 23 2021, 11:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2021, 11:51 AM

This is pushing over the edge of pattern-matching complexity. But again, I don't have any real suggestions for improvement, so adding more potential reviewers.

foad added a comment.Jan 6 2022, 1:17 AM

I don't really have an opinion on the patch, but I'm curious.

Given (~a & ~b & c) | (~a & ~c & b), do we make any attempt to pull out the ~a like this: ~a & ((~b & c) | (~c & b)) ? If not, why not? Would that be a good thing to address? Then it's just a case of simplifying the two-variable (~b & c) | (~c & b) -> b ^ c which is easy.

spatel added a comment.Jan 6 2022, 7:33 AM

I don't really have an opinion on the patch, but I'm curious.

Given (~a & ~b & c) | (~a & ~c & b), do we make any attempt to pull out the ~a like this: ~a & ((~b & c) | (~c & b)) ? If not, why not? Would that be a good thing to address? Then it's just a case of simplifying the two-variable (~b & c) | (~c & b) -> b ^ c which is easy.

InstCombine does try that transform via InstCombinerImpl::SimplifyAssociativeOrCommutative() and/or InstCombinerImpl::SimplifyUsingDistributiveLaws(). And that works as expected - for example if we alter the 1st modified test in this patch to be like this:

define i32 @not_and_not_and_and_or(i32 %a, i32 %b, i32 %c) {
  %nota = xor i32 %a, -1
  %notb = xor i32 %b, -1
  %and1 = and i32 %nota, %c
  %and2 = and i32 %and1, %notb
  %or1 = or i32 %a, %c
  %not1 = xor i32 %or1, -1
  %and3 = and i32 %not1, %b
  %or3 = or i32 %and2, %and3
  call void @use(i32 %nota)
  call void @use(i32 %notb)
  ret i32 %or3

Then it reduces because we find the 'b' and '~b' values directly in the operands of the 'and' instructions. So this might be a question for "-reassociate" - can we get that pass to arrange the operands such that -instcombine can fold this (without breaking some other pattern)?

I don't really have an opinion on the patch, but I'm curious.

Given (~a & ~b & c) | (~a & ~c & b), do we make any attempt to pull out the ~a like this: ~a & ((~b & c) | (~c & b)) ? If not, why not? Would that be a good thing to address? Then it's just a case of simplifying the two-variable (~b & c) | (~c & b) -> b ^ c which is easy.

InstCombine does try that transform via InstCombinerImpl::SimplifyAssociativeOrCommutative() and/or InstCombinerImpl::SimplifyUsingDistributiveLaws(). And that works as expected - for example if we alter the 1st modified test in this patch to be like this:

define i32 @not_and_not_and_and_or(i32 %a, i32 %b, i32 %c) {
  %nota = xor i32 %a, -1
  %notb = xor i32 %b, -1
  %and1 = and i32 %nota, %c
  %and2 = and i32 %and1, %notb
  %or1 = or i32 %a, %c
  %not1 = xor i32 %or1, -1
  %and3 = and i32 %not1, %b
  %or3 = or i32 %and2, %and3
  call void @use(i32 %nota)
  call void @use(i32 %notb)
  ret i32 %or3

Then it reduces because we find the 'b' and '~b' values directly in the operands of the 'and' instructions. So this might be a question for "-reassociate" - can we get that pass to arrange the operands such that -instcombine can fold this (without breaking some other pattern)?

Ressociate does that and in fact a simpler case works. The problem is with a more complex case where other subexpression is used elsewhere. For example if (~a & b) is used. In that case reassociate does not do it because it is not beneficial, and it only becomes beneficial when we consider a bigger pattern.

In fact the original motivating case has all possible subexpression permutations in its 255 logical functions, which then lead to mutlti-use cases.

rampitec abandoned this revision.Feb 3 2022, 2:04 PM