Page MenuHomePhabricator

[instcombine] Fold overflow check using umulo to comparison
ClosedPublic

Authored by reames on Jun 21 2021, 12:51 PM.

Details

Summary

If we have a umul.with.overflow where the multiply result is not used and one of the operands is a constant, we can perform the overflow check cheaper with a comparison then by performing the multiply an extracting the overflow flag.

(Noticed when looking at the conditions SCEV emits for overflow checks.)

Diff Detail

Event Timeline

reames created this revision.Jun 21 2021, 12:51 PM
reames requested review of this revision.Jun 21 2021, 12:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2021, 12:51 PM

Seems right- see inline for a few nits along with the clang-format warnings from the bot.
Please pre-commit the test file with the baseline CHECKs.

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

Add assert msg - "Unexpected extract index for overflow inst"

3088

This comment is too specific now - we handle more than just adds.

// If we are only checking for overflow (math result is not used) and the RHS is a constant...
3105

This matches the existing check for add, but we could use match(WO->getRHS(), m_APInt(C)) and handle vector splat constants for almost free. Add a TODO comment?

reames updated this revision to Diff 353811.Jun 22 2021, 3:50 PM

Address review comments

reames updated this revision to Diff 353813.Jun 22 2021, 3:57 PM

Add requested todo.

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

Done, though I don't think this actually improves the code.

3105

I hadn't know these had vector overloads. Learn something new every day.

spatel accepted this revision.Jun 23 2021, 8:56 AM

LGTM

This revision is now accepted and ready to land.Jun 23 2021, 8:56 AM
This revision was landed with ongoing or failed builds.Jun 25 2021, 10:26 AM
This revision was automatically updated to reflect the committed changes.