Page MenuHomePhabricator

[InstCombine] Optimize overflow check base on uadd.with.overflow result (PR40846)
AcceptedPublic

Authored by nikic on Feb 25 2019, 1:08 PM.

Details

Summary

Fix for https://bugs.llvm.org/show_bug.cgi?id=40846.

This adds a combine for cases where a (a + b) < a style overflow check is performed, but with a + b being the result of uadd.with.overflow, so the overflow result is also already available and we can just use it. Subsequently GVN/CSE will deduplicate the extracts.

We can run into this situation if you have both a uadd.with.overflow and a manual add + overflow check in the same function (on the same operands), in which case GVN will rewrite the add to the with.overflow result and leave you with this pattern.

The implementation is a bit ugly because I'm handling the various canonicalization edge cases. I'm wondering if we couldn't introduce something like m_ICmpULT() to match both ULT and the UGT commutation as well as the canonicalization edge-cases for constant operands.

Diff Detail

Event Timeline

nikic created this revision.Feb 25 2019, 1:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2019, 1:08 PM
alex added a subscriber: alex.Feb 25 2019, 1:10 PM
spatel added a comment.Mar 4 2019, 3:32 PM

Probably should not be included in the same patch, but if we're seeing this pattern, then is it likely that we'd also see code that checks the inverse condition (that the overflow did not occur)?

define i1 @uadd_res_uge_x(i32 %x, i32 %y, i1* %p) {
  %a = tail call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 %x, i32 %y)
  %b = extractvalue { i32, i1 } %a, 1
  store i1 %b, i1* %p, align 1
  %c = extractvalue { i32, i1 } %a, 0
  %d = icmp uge i32 %c, %x
  ret i1 %d
}
nikic updated this revision to Diff 227603.Sun, Nov 3, 2:21 AM
nikic added a reviewer: lebedev.ri.
nikic changed the repository for this revision from rL LLVM to rG LLVM Github Monorepo.

Rebase and use new matchers.

spatel accepted this revision.Tue, Nov 5, 10:06 AM

LGTM

This revision is now accepted and ready to land.Tue, Nov 5, 10:06 AM
lebedev.ri accepted this revision.Tue, Nov 5, 10:30 AM

This LG, thanks.

Forgot to add, what about other predicates +xor (extract %agg, 1), -1 ?