From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x930.google.com (mail-ua1-x930.google.com [IPv6:2607:f8b0:4864:20::930]) by sourceware.org (Postfix) with ESMTPS id C66103858C5E for ; Wed, 14 Jun 2023 14:46:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C66103858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ua1-x930.google.com with SMTP id a1e0cc1a2514c-789c56ead4fso932818241.1 for ; Wed, 14 Jun 2023 07:46:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1686753960; x=1689345960; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=jvIaQ2t61GtQEaUxUKrtbxtkIZ7r/npZ2QWLk14GNUo=; b=J5SOorBycfdXR8DPk0qHRK5JPvkPzeVGYKB1GTa8zyr+WZ9opAnJxQ1CK1hnmbTTvt J1kGLMJd3alkfP8PNn+AHeBu6mjUz5axkgVnCnBGWeWSUsEjzE8hbRpnGn24gbDRG0mh pI6KCileKWH01+rckUvmAhVUDVq3Spq+zXLEcLO7nj7umU8fN21NgUpP61craj7/x+9D 5mtBksEZ50KW9dniSdLTGPoN5ZKEz5LidJcFLSHO68gpYmZLCWPNCE45UPUoYQV5MtRm 4gC7omBC5jb8aq8RM97XRaQYcco+TpG3LcM38+vNpIDBxTU+KYbz7xcgl9vLls4l8pvd /ucA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686753960; x=1689345960; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jvIaQ2t61GtQEaUxUKrtbxtkIZ7r/npZ2QWLk14GNUo=; b=G1VioBZAuiWS8jNWvGOJRmu0LLtT1cOyF65UZ+a3G4VRoFEIFXH70E/PCVtitOFz5+ vZZzfQSdqR3M91J9jzP7KfWEy+LwupoevFEqbAvuw8OEdf4wGXbAP2LfglVkR3kX9LlT iO6VT9SIRshwfJNbJBSb9xN+dW7r4tpxosu0RyjLqAdIZ3awiNVMpikXaNm0pSk2VCoG 2JICKXzL6FK754ysJl0P19iA8OO/5GCpCM/stKFmh4YUi8NbmfCNe6FIf0TNgQIga5fP mGC2TlgPip5kqRYoopiWcvC907YWszHkpVdym9Wh1CIdqBf+jb138BHC30iOXA6O96jq zKlQ== X-Gm-Message-State: AC+VfDxzdcl2I3uzTQkRH7Q/e+PkXYj1S1ag6GcAOwoUxifbyf5en2fG fvQy4fyvN5X3eRnoBFrOq2odDXI9YOMdY9oHxDY= X-Google-Smtp-Source: ACHHUZ5LrtrwUiPqAv+akIAKCfM/x0WdSE9xWODbN1QpnYqL9w5eIuFlXKv3nKyFZFLVCSYjgEcuc92QTOPhdUpqPt8= X-Received: by 2002:a1f:5e8f:0:b0:46e:97e8:f68c with SMTP id s137-20020a1f5e8f000000b0046e97e8f68cmr903949vkb.2.1686753960036; Wed, 14 Jun 2023 07:46:00 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Uros Bizjak Date: Wed, 14 Jun 2023 16:45:48 +0200 Message-ID: Subject: Re: [PATCH] middle-end, i386, v3: Pattern recognize add/subtract with carry [PR79173] To: Jakub Jelinek Cc: Richard Biener , Roger Sayle , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, Jun 14, 2023 at 4:00=E2=80=AFPM Jakub Jelinek wr= ote: > > Hi! > > On Wed, Jun 14, 2023 at 12:35:42PM +0000, Richard Biener wrote: > > At this point two pages of code without a comment - can you introduce > > some vertical spacing and comments as to what is matched now? The > > split out functions help somewhat but the code is far from obvious :/ > > > > Maybe I'm confused by the loops and instead of those sth like > > > > if (match_x_y_z (op0) > > || match_x_y_z (op1)) > > ... > > > > would be easier to follow with the loop bodies split out? > > Maybe put just put them in lambdas even? > > > > I guess you'll be around as long as myself so we can go with > > this code under the premise you're going to maintain it - it's > > not that I'm writing trivially to understand code myself ... > > As I said on IRC, I don't really know how to split that into further > functions, the problem is that we need to pattern match a lot of > statements and it is hard to come up with names for each of them. > And we need quite a lot of variables for checking their interactions. > > The code isn't that much different from say match_arith_overflow or > optimize_spaceship or other larger pattern recognizers. And the > intent is that all the code paths in the recognizer are actually covered > by the testcases in the testsuite. > > That said, I've added 18 new comments to the function, and rebased it > on top of the > https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621717.html > patch with all constant arguments handling moved to fold-const-call.cc > even for the new ifns. > > Ok for trunk like this if it passes bootstrap/regtest? > > 2023-06-13 Jakub Jelinek > > PR middle-end/79173 > * internal-fn.def (UADDC, USUBC): New internal functions. > * internal-fn.cc (expand_UADDC, expand_USUBC): New functions. > (commutative_ternary_fn_p): Return true also for IFN_UADDC. > * optabs.def (uaddc5_optab, usubc5_optab): New optabs. > * tree-ssa-math-opts.cc (uaddc_cast, uaddc_ne0, uaddc_is_cplxpart= , > match_uaddc_usubc): New functions. > (math_opts_dom_walker::after_dom_children): Call match_uaddc_usub= c > for PLUS_EXPR, MINUS_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR unless > other optimizations have been successful for those. > * gimple-fold.cc (gimple_fold_call): Handle IFN_UADDC and IFN_USU= BC. > * fold-const-call.cc (fold_const_call): Likewise. > * gimple-range-fold.cc (adjust_imagpart_expr): Likewise. > * tree-ssa-dce.cc (eliminate_unnecessary_stmts): Likewise. > * doc/md.texi (uaddc5, usubc5): Document new named > patterns. > * config/i386/i386.md (subborrow): Add alternative with > memory destination. > (uaddc5, usubc5): New define_expand patterns. > (*sub_3, @add3_carry, addcarry, @sub3_car= ry, > subborrow, *add3_cc_overflow_1): Add define_peephole2 > TARGET_READ_MODIFY_WRITE/-Os patterns to prefer using memory > destination in these patterns. > > * gcc.target/i386/pr79173-1.c: New test. > * gcc.target/i386/pr79173-2.c: New test. > * gcc.target/i386/pr79173-3.c: New test. > * gcc.target/i386/pr79173-4.c: New test. > * gcc.target/i386/pr79173-5.c: New test. > * gcc.target/i386/pr79173-6.c: New test. > * gcc.target/i386/pr79173-7.c: New test. > * gcc.target/i386/pr79173-8.c: New test. > * gcc.target/i386/pr79173-9.c: New test. > * gcc.target/i386/pr79173-10.c: New test. +;; Helper peephole2 for the addcarry and subborrow +;; peephole2s, to optimize away nop which resulted from uaddc/usubc +;; expansion optimization. +(define_peephole2 + [(set (match_operand:SWI48 0 "general_reg_operand") + (match_operand:SWI48 1 "memory_operand")) + (const_int 0)] + "" + [(set (match_dup 0) (match_dup 1))]) Is this (const_int 0) from a recent patch from Roger that introduced: +;; Set the carry flag from the carry flag. +(define_insn_and_split "*setccc" + [(set (reg:CCC FLAGS_REG) + (reg:CCC FLAGS_REG))] + "ix86_pre_reload_split ()" + "#" + "&& 1" + [(const_int 0)]) + +;; Set the carry flag from the carry flag. +(define_insn_and_split "*setcc_qi_negqi_ccc_1_" + [(set (reg:CCC FLAGS_REG) + (ltu:CCC (reg:CC_CCC FLAGS_REG) (const_int 0)))] + "ix86_pre_reload_split ()" + "#" + "&& 1" + [(const_int 0)]) + +;; Set the carry flag from the carry flag. +(define_insn_and_split "*setcc_qi_negqi_ccc_2_" + [(set (reg:CCC FLAGS_REG) + (unspec:CCC [(ltu:QI (reg:CC_CCC FLAGS_REG) (const_int 0)) + (const_int 0)] UNSPEC_CC_NE))] + "ix86_pre_reload_split ()" + "#" + "&& 1" + [(const_int 0)]) If this interferes with RTL stream, then instead of emitting (const_int 0), the above patterns should simply emit: { emit_note (NOTE_INSN_DELETED); DONE; } And there will be no (const_int 0) in the RTL stream. Uros.