This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Improve foldOpIntoPhi() to use isImpliedCondition
ClosedPublic

Authored by kitaisreal on Jul 30 2023, 8:14 AM.

Diff Detail

Event Timeline

kitaisreal created this revision.Jul 30 2023, 8:14 AM
kitaisreal requested review of this revision.Jul 30 2023, 8:14 AM
nikic added inline comments.Jul 30 2023, 9:09 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1393

There is an isImpliedByCondition() overload that accepts unpacked icmp operands. We should use that instead of cloning the instruction.

Updated implementation.

kitaisreal marked an inline comment as done.Jul 30 2023, 11:44 AM
nikic added inline comments.Jul 30 2023, 12:51 PM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1388

Please extract the code above here into a separate function (which does simplifyInstructionWithOperands plus the special icmp case).

1394

I don't think the RHSOp0, RHSOp1 variables really add value over Ops[0], Ops[1] here.

1395

I think your tests currently only cover one of the LHSIsTrue cases. You also need one with successors swapped.

1402

You can use ConstantInt::getBool() here.

goldstein.w.n added inline comments.Jul 30 2023, 2:47 PM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1400

can rename variable from 'Constant'. Its a big confusing to have variable name ==type.

kitaisreal marked 5 inline comments as done.

Updated implementation.

Looks basically fine to me.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
1338

I think this should check that getSuccessor(0) != getSuccessor(1). This will get canonicalized away, but is not guaranteed due to worklist order.

1345

I'd personally move this block to the end, because simplifyInstructionWithOperands() is the normal case, and this is a special case.

1355

Omit braces

kitaisreal marked 3 inline comments as done.

Updated implementation.

nikic accepted this revision.Jul 31 2023, 11:48 AM

LGTM

This revision is now accepted and ready to land.Jul 31 2023, 11:48 AM