[PATCH] D51929: [DAGCombiner] use UADDO to optimize saturated unsigned add

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 11 08:25:39 PDT 2018


spatel added a comment.

> That's why AArch64 only shows diffs for i32/i64.

Reading that again, that's likely to confuse you. :) More detailed explanation:

AArch shows 'uaddo' codegen for the i8/i16/i32/i64 test variants with "using_cmp_sum" in the title. That's the pattern that CGP matches as an unsigned saturated add and converts to uaddo without checking target capabilities.

This patch is gated by isOperationLegalOrCustom(ISD::UADDO, VT), so we see only see AArch diffs for i32/i64 in the tests with "using_cmp_notval" in the title (unlike x86 which sees improvements for all sizes because all sizes are 'custom'). But the AArch code (like x86) looks better when translated to 'uaddo' in all cases. So someone that is involved with AArch may want to set i8/i16 to 'custom' for UADDO, so this patch will fire on those tests.

Another possibility given the existing behavior: we could remove the legal-or-custom check altogether because we're assuming that a UADDO sequence is canonical/optimal before we ever reach here. But that seems like a bug to me. If the target doesn't have an add-with-flags op, then it's not likely that we'll get optimal DAG combining using a UADDO node. This is similar justification for why we don't canonicalize IR to the overflow math intrinsic sibling (llvm.uadd.with.overflow) for UADDO in the first place.


https://reviews.llvm.org/D51929





More information about the llvm-commits mailing list