From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by sourceware.org (Postfix) with ESMTPS id 86C723858D28 for ; Mon, 24 Apr 2023 11:59:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 86C723858D28 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-lj1-x233.google.com with SMTP id 38308e7fff4ca-2a8a600bd05so40462541fa.2 for ; Mon, 24 Apr 2023 04:59:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682337539; x=1684929539; 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=SHl1/Rx4RM15j4BJ+wTWpnQ1hr92MGnChYh6bzAI16o=; b=G1gDI+eFQ/5e31YWEfpLY2i/Vo2sScgdqzCbPJ+ODFisNR6ekim0TKsy8kUGltglVY h1H4Y4xG3YGuqwVeKhCRR9nLKJc+eJHM3BdcAg3ihpnhwZHPleNlxoPWWrKCQpbyMtBv cBLUmx747YoLb0DJwsXkt84z0iEeGsU30LsFDuGDEsE111nxiIfOiFT8MeC3fyIsyMxE hoIN0t5hYsFPFWskbfXbJVAJSV6WoZXn4oKL00cxZfvxTO49zCw/Ss9SKehvXDwD9Krp fB8Xryr7wOAi92+Hz8E5xYjN7rAF/qE/i7djaMrxtXijp/zwzoYcqUP5CWB5XxF9/vqU tuZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682337539; x=1684929539; 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=SHl1/Rx4RM15j4BJ+wTWpnQ1hr92MGnChYh6bzAI16o=; b=MYxl+D98NcrXhLg2HqwH/XJM/skIWSGss8mWTTxOgevE6gmN4porzk2qmxMmWJNNd0 vsyk1XKIKb78b0lA137xIjUlYBEkvtMWuOgNrO708fCA8JjI3AZxqlDphpv6hpvRKDNs qhD+ho2HrYYRb2G532anPuIbR8wDlPm5mlMMS5FZJi1qL6lqAFXd/mJC5C4a6yHag9yS CbsoEsL+Dl0l+NmuG/S/Z/7zNhH3b4zCcVr91BJWSfj0UNXdQmQEcVLWhn1SlgAAosc1 oObgS2bz/2M+u3HaAafWHjcaRS2SBFU7ozpJGRxUTvzCFzl/Btf35QI4rS+EXY9EQEya DULg== X-Gm-Message-State: AAQBX9cho7NGa5LKW2rgNNZRbNzDqhw5b/Q7XI7K/C+23aEdfKqvdeQQ eB0kX0suXnglgULNksWk5XezfTEtd5j7mF2Ue2A= X-Google-Smtp-Source: AKy350ahGJJ/4OVVb950vnHa4yaxgYwoYM2moAAJFLNf3vuw8YIK67ePoxYpl5P3VBO0AJGGLdAsYANMal5BhLrG1aw= X-Received: by 2002:a05:651c:10c2:b0:2a8:c842:d30a with SMTP id l2-20020a05651c10c200b002a8c842d30amr2259026ljn.37.1682337538766; Mon, 24 Apr 2023 04:58:58 -0700 (PDT) MIME-Version: 1.0 References: <51ce8969-3130-452e-092e-f9d91eff2dad@arm.com> In-Reply-To: From: Richard Biener Date: Mon, 24 Apr 2023 13:57:33 +0200 Message-ID: Subject: Re: [ping][vect-patterns] Refactor widen_plus/widen_minus as internal_fns To: "Andre Vieira (lists)" Cc: Richard Biener , Richard Sandiford , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,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 Thu, Apr 20, 2023 at 3:24=E2=80=AFPM Andre Vieira (lists) via Gcc-patche= s wrote: > > Rebased all three patches and made some small changes to the second one: > - removed sub and abd optabs from commutative_optab_p, I suspect this > was a copy paste mistake, > - removed what I believe to be a superfluous switch case in vectorizable > conversion, the one that was here: > + if (code.is_fn_code ()) > + { > + internal_fn ifn =3D as_internal_fn (code.as_fn_code ()); > + int ecf_flags =3D internal_fn_flags (ifn); > + gcc_assert (ecf_flags & ECF_MULTI); > + > + switch (code.as_fn_code ()) > + { > + case CFN_VEC_WIDEN_PLUS: > + break; > + case CFN_VEC_WIDEN_MINUS: > + break; > + case CFN_LAST: > + default: > + return false; > + } > + > + internal_fn lo, hi; > + lookup_multi_internal_fn (ifn, &lo, &hi); > + *code1 =3D as_combined_fn (lo); > + *code2 =3D as_combined_fn (hi); > + optab1 =3D lookup_multi_ifn_optab (lo, !TYPE_UNSIGNED (vectype)); > + optab2 =3D lookup_multi_ifn_optab (hi, !TYPE_UNSIGNED (vectype)); > } > > I don't think we need to check they are a specfic fn code, as we look-up > optabs and if they succeed then surely we can vectorize? > > OK for trunk? In the first patch I see some uses of safe_as_tree_code like + if (ch.is_tree_code ()) + return op1 =3D=3D NULL_TREE ? gimple_build_assign (lhs, ch.safe_as_tree_code (), + op0) : + gimple_build_assign (lhs, ch.safe_as_tree_cod= e (), + op0, op1); + else + { + internal_fn fn =3D as_internal_fn (ch.safe_as_fn_code ()); + gimple* stmt; where the context actually requires a valid tree code. Please change those to force to tree code / ifn code. Just use explicit casts here and the oth= er places that are similar. Before the as_internal_fn just put a gcc_assert (ch.is_internal_fn ()). Maybe the need for the (ugly) safe_as_tree_code/fn_code goes away then. Otherwise patch1 looks OK. Unfortunately there are no ChangeLog / patch descriptions on the changes. patch2 has - tree_code rhs_code =3D gimple_assign_rhs_code (assign); - if (rhs_code !=3D code && rhs_code !=3D widened_code) + code_helper rhs_code; + if (is_gimple_assign (stmt)) + { + rhs_code =3D gimple_assign_rhs_code (stmt); + if (rhs_code.safe_as_tree_code () !=3D code + && rhs_code.get_rep () !=3D widened_code.get_rep ()) + return 0; + } + else if (is_gimple_call (stmt)) + { + rhs_code =3D gimple_call_combined_fn (stmt); + if (rhs_code.get_rep () !=3D widened_code.get_rep ()) + return 0; + } that looks mightly complicated - esp. the use of get_rep () looks dangerous? What's the intent of this? Not that I understand the existing code much. A comment would clearly help (also indicating test coverage). > Kind regards, > Andre