This is an archive of the discontinued LLVM Phabricator instance.

Canonicalize guards for AND condition
ClosedPublic

Authored by mkazantsev on Jan 24 2017, 2:52 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

This revision is now accepted and ready to land.Jan 24 2017, 2:54 AM
mkazantsev requested review of this revision.Jan 24 2017, 2:54 AM
mkazantsev edited edge metadata.
apilipenko added inline comments.Jan 24 2017, 9:51 AM
lib/Transforms/InstCombine/InstCombineCalls.cpp
2883–2884 ↗(On Diff #85552)

New guard ...

2885 ↗(On Diff #85552)

Minor:

Function *GuardIntrinsic = II->getCalledFunction();
2888–2891 ↗(On Diff #85552)

We might have a guard with non-standard calling convention. I guess you'll need to set the cc for new guards as well.

mkazantsev updated this revision to Diff 85687.Jan 24 2017, 7:47 PM
mkazantsev marked 3 inline comments as done.

Done fixes

apilipenko added inline comments.Jan 25 2017, 12:28 AM
lib/Transforms/InstCombine/InstCombineCalls.cpp
2889–2894 ↗(On Diff #85691)

Maybe:

auto CC = II->getCallingConv()
Builder->CreateCall(A)->setCallingConv(CC)
Builder->CreateCall(B)->setCallingConv(CC)
test/Transforms/InstCombine/call-guard.ll
32 ↗(On Diff #85691)

Please add a test with non-default calling convention

mkazantsev added inline comments.Jan 25 2017, 12:59 AM
lib/Transforms/InstCombine/InstCombineCalls.cpp
2889–2894 ↗(On Diff #85691)

I would rather not do call creation and setting the convention in one line, because in case if we want to do to them something more than setting the CC in future, this code will need to be changed.

mkazantsev updated this revision to Diff 85710.Jan 25 2017, 1:22 AM
mkazantsev marked 2 inline comments as done.

Added test for non-standard CC

mkazantsev marked an inline comment as done.Jan 25 2017, 1:23 AM
apilipenko accepted this revision.Jan 25 2017, 1:46 AM
apilipenko added inline comments.
lib/Transforms/InstCombine/InstCombineCalls.cpp
2889–2894 ↗(On Diff #85691)

Not a big deal IMO, but I leave it up to you.

test/Transforms/InstCombine/call-guard.ll
42 ↗(On Diff #85710)

@test_guard_and_non_default_cc? There is nothing specific about cc99 in this test.

This revision is now accepted and ready to land.Jan 25 2017, 1:46 AM
mkazantsev updated this revision to Diff 85718.Jan 25 2017, 2:09 AM
mkazantsev marked 2 inline comments as done.

Reformatted, renamed test

This revision was automatically updated to reflect the committed changes.