Example https://godbolt.org/z/acq4rqdqK.
Depends on D156619.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| 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. | |
| 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. | |
| llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | ||
|---|---|---|
| 1400 | can rename variable from 'Constant'. Its a big confusing to have variable name ==type. | |
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 | |
I think this should check that getSuccessor(0) != getSuccessor(1). This will get canonicalized away, but is not guaranteed due to worklist order.