This is an archive of the discontinued LLVM Phabricator instance.

Added InstCombine Transform for ((B | C) & A) | B -> B | (A & C)
ClosedPublic

Authored by sonamkumari on Aug 12 2014, 4:13 AM.

Details

Summary

Hi All,

This patch implements transform for the pattern " ((B | C) & A) | B -> B | (A & C) "

Please help in reviewing the same.

Z3 Link : http://rise4fun.com/Z3/hP6p

Thanks,
Sonam.

Diff Detail

Event Timeline

sonamkumari retitled this revision from to Added InstCombine Transform for ((B | C) & A) | B -> B | (A & C).
sonamkumari updated this object.
sonamkumari edited the test plan for this revision. (Show Details)
sonamkumari added a subscriber: Unknown Object (MLST).
majnemer edited edge metadata.Aug 12 2014, 9:18 AM

While this seems to handle ((B | C) & A) | B, it would be nice if you also handled ((B | C) & A) | C

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
2129

This is a longwinded way of saying Op1 == B.

sonamkumari edited edge metadata.

Hi David,

Thanks for your valuable comments.
I have changed the second "if" condition as you mentioned.
For handling ( ( B | C ) & A ) | C -> C | ( A & B) :
I passed the following test case using reassociate before instcombine

define i32 @test19(i32 %x, i32 %y, i32 %z) {

%or = or i32 %y, %z
%and = and i32 %x, %or
%or1 = or i32 %and, %z
ret i32 %or1

}

The output I got was :

define i32 @test19(i32 %x, i32 %y, i32 %z) {

%1 = and i32 %x, %y
%or1 = or i32 %1, %z
ret i32 %or1

}

So, I guess we need not handle this case.
Please help in reviewing the same.

Thanks,
Sonam.

majnemer added inline comments.Aug 12 2014, 11:08 PM
lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
2128–2129

Hrm, maybe this could be simplified further:

if (match(Op0, m_And(m_Or(m_Specific(Op1), m_Value(C)), m_Value(A))))
  return BinaryOperator::CreateOr(Op1, Builder->CreateAnd(A, C));

Hi David,

Thanks for further simplifying it.
I have made the changes and submitted the same.
Please help in reviewing the same.

Thanks,
Sonam

majnemer accepted this revision.Aug 13 2014, 4:03 PM
majnemer edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Aug 13 2014, 4:03 PM

Hi David,

Can you please commit this on my behalf as i don't have commit access.

Thanks,
Sonam.

majnemer closed this revision.Aug 13 2014, 11:50 PM
majnemer updated this revision to Diff 12488.

Closed by commit rL215619 (authored by @majnemer).