Page MenuHomePhabricator

Added InstCombine transform for pattern "(A ^ B) & ((~A) ^ B) -> False"
Needs ReviewPublic

Authored by sonamkumari on Jul 27 2014, 11:36 PM.

Details

Summary

This patch implements transform for pattern "(A ^ B) & ((~A) ^ B) -> False".

Please help in reviewing the same.

Thanks,
Sonam.

Diff Detail

Event Timeline

sonamkumari retitled this revision from to Added InstCombine transform for pattern "(A ^ B) & ((~A) ^ B) -> False".
sonamkumari updated this object.
sonamkumari edited the test plan for this revision. (Show Details)
sonamkumari added a subscriber: Unknown Object (MLST).
majnemer added inline comments.
lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
1287–1290

Please format the code to match the coding standards: http://llvm.org/docs/CodingStandards.html

1290

This is wasteful, we don't need an instruction if we know the result is a constant. Sounds like a candidate for InstSimplify instead of InstCombine.

test/Transforms/InstCombine/or-xor.ll
96–104 ↗(On Diff #11937)

Please format this test to match the others.

sonamkumari updated this revision to Diff 12191.Aug 5 2014, 3:19 AM
sonamkumari added a reviewer: majnemer.

Hi David,

Thanks for your valuable comments.I will try to consider general case while doing any optimization in future.
But the patch which you uploaded in rL214342 fails for the following test cases as it doesn't take into consideration the symmetric case.

The test cases for which it fails are :

define i32 @test15(i32 %x, i32 %y) {
%nega = xor i32 %x, -1
%xor = xor i32 %nega, %y
%xor1 = xor i32 %x, %y
%and = and i32 %xor, %xor1
ret i32 %and
}

define i32 @test16(i32 %x, i32 %y) {
%xor = xor i32 %x, %y
%nega = xor i32 %x, -1
%xor1 = xor i32 %nega, %y
%and = and i32 %xor, %xor1
ret i32 %and
}

So, I modified the patch and submitting the same.
Kindly review it.

Thanks,
Sonam.

suyog added a reviewer: suyog.Aug 7 2014, 4:41 AM
suyog edited edge metadata.

LGTM. I would wait for David to comment on it though.

majnemer edited edge metadata.Aug 7 2014, 11:04 PM

I don't think we need to do anything here, running the reassociate pass before instcombine makes this problem go away.

$ cat t.ll
define i32 @test15(i32 %x, i32 %y) {
 %nega = xor i32 %x, -1
 %xor = xor i32 %nega, %y
 %xor1 = xor i32 %x, %y
 %and = and i32 %xor, %xor1
 ret i32 %and
}

define i32 @test16(i32 %x, i32 %y) {
 %xor = xor i32 %x, %y
 %nega = xor i32 %x, -1
 %xor1 = xor i32 %nega, %y
 %and = and i32 %xor, %xor1
 ret i32 %and
}

$ ~/llvm/Debug+Asserts/bin/opt -reassociate -instcombine t.ll -o - -S
; ModuleID = 't.ll'

define i32 @test15(i32 %x, i32 %y) {
  ret i32 0
}

define i32 @test16(i32 %x, i32 %y) {
  ret i32 0
}
silvas resigned from this revision.Jul 8 2016, 11:38 PM
silvas removed a reviewer: silvas.
dexonsmith resigned from this revision.Jul 11 2016, 6:42 PM
dexonsmith removed a reviewer: dexonsmith.
espindola edited reviewers, added: espindola; removed: rafael.Mar 15 2018, 11:06 AM