From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) by sourceware.org (Postfix) with ESMTPS id 3B1263857714 for ; Mon, 3 Jul 2023 08:46:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3B1263857714 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-x230.google.com with SMTP id 38308e7fff4ca-2b69ea3b29fso65291481fa.3 for ; Mon, 03 Jul 2023 01:46:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688373963; x=1690965963; 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=gIIZawKnYcPmJ28y/Owi7y63d9hp1E0iohw/1sFxfog=; b=Sq1w4ZwZIe4ovajwT/mN0MLaBpcyOdV9us5pyPMvq6ezRZHMxXkwOT5GmybRyr6PUt GEai9ZEn4zTvgy5L3jnSw+E+q8wf4aUfJ7IDoxm0QMM2Saqy08QgZ0c/Pg9Bcz1wKcQq 8YlpgNAbV25kBY/tE4kiOLRo4s175qeaCZZbP+0mgNAx8pGXoiLDx9/U/P/oLxJ+CooP OPDleyiXx9rQr9gSCLY/5nWMarglWTyXgvbcQjRx1Dh4ybGlLaplkiLQQwmyJbYPIn5s v9s2Gqnwy16XXaT9NkSwODl+9FmotU4uwrh1g/VI1GUU9ULo5Wq+rWZj1c8bIyZAO9Lr leyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688373963; x=1690965963; 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=gIIZawKnYcPmJ28y/Owi7y63d9hp1E0iohw/1sFxfog=; b=ejR1ptFJ+82oD1tPXS9gZRLgBUem2r5dblsAy/VA4SrxQvdtKo5Ky3gW0cmNftIvEG CSjElObCZh6QL+5zihBqj498+0kHuVl2i7ihvmdUIrPJuhdjOVbiwpk1ptHXIzrnI5Df otZVmzZxd+tcwUGe33Cz2wbM2OBIdjLv6SdeINq4KH4hFJ5kHgxlI++G5osQcJcRp7J+ G0TKhmz/H8GtBnIbMlBUKn+RLP4QvOwG+GqZYZsUqWNMbdB7Hd/nIM+wVBD+sYLbX4lu t/aD2UndvX1Msp+CgcAC4tI+KQ3lUkHYt07rr7tdh4IKLO5SSAWt+qYmco2ZVL4ja9Yv rqSQ== X-Gm-Message-State: ABy/qLbgrhf+dP3uw7VUx7evIzuHA0vkPDXCyKNlV36pqmyoltg6TStF nZ/88LSazpk4Nv5o0wRIiD2onIledQmvJnzHYRs= X-Google-Smtp-Source: APBJJlG0lenShr8ERKOqVuOrU0Y5viElO5kAisM4/OJsTLQEc92lNVSnEtNthcz5tzGcUs3xE1vkWmGp/OeCIDk7UMM= X-Received: by 2002:a2e:800b:0:b0:2b6:c886:681 with SMTP id j11-20020a2e800b000000b002b6c8860681mr5767757ljg.6.1688373962555; Mon, 03 Jul 2023 01:46:02 -0700 (PDT) MIME-Version: 1.0 References: <12ec1904-ad86-2bb3-2f19-e3651a7fb492@linux.ibm.com> In-Reply-To: <12ec1904-ad86-2bb3-2f19-e3651a7fb492@linux.ibm.com> From: Richard Biener Date: Mon, 3 Jul 2023 10:42:48 +0200 Message-ID: Subject: Re: [PATCH 0/9] vect: Move costing next to the transform for vect load To: "Kewen.Lin" Cc: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com, segher@kernel.crashing.org, bergner@linux.ibm.com 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 Mon, Jul 3, 2023 at 5:39=E2=80=AFAM Kewen.Lin wrot= e: > > Hi Richi, > > Thanks for your review comments on this and some others! > > on 2023/6/30 19:37, Richard Biener wrote: > > On Tue, Jun 13, 2023 at 4:07=E2=80=AFAM Kewen Lin = wrote: > >> > >> This patch series follows Richi's suggestion at the link [1], > >> which suggest structuring vectorizable_load to make costing > >> next to the transform, in order to make it easier to keep > >> costing and the transform in sync. For now, it's a known > >> issue that what we cost can be inconsistent with what we > >> transform, as the case in PR82255 and some other associated > >> test cases in the patches of this series show. > >> > >> Basically this patch series makes costing not call function > >> vect_model_load_cost any more. To make the review and > >> bisection easy, I organized the changes according to the > >> memory access types of vector load. For each memory access > >> type, firstly it follows the handlings in the function > >> vect_model_load_costto avoid any missing, then refines > >> further by referring to the transform code, I also checked > >> them with some typical test cases to verify. Hope the > >> subjects of patches are clear enough. > >> > >> The whole series can be bootstrapped and regtested > >> incrementally on: > >> - x86_64-redhat-linux > >> - aarch64-linux-gnu > >> - powerpc64-linux-gnu P7, P8 and P9 > >> - powerpc64le-linux-gnu P8, P9 and P10 > >> > >> By considering the current vector test buckets are mainly > >> tested without cost model, I also verified the whole patch > >> series was neutral for SPEC2017 int/fp on Power9 at O2, > >> O3 and Ofast separately. > > > > I went through the series now and I like it overall (well, I suggested > > the change). > > Looking at the changes I think we want some followup to reduce the > > mess in the final loop nest. We already have some VMAT_* cases handled > > separately, maybe we can split out some more cases. Maybe we should > > At first glance, the simple parts look to be the handlings for > VMAT_LOAD_STORE_LANES, and VMAT_GATHER_SCATTER (with ifn and emulated). > It seems a bit straightforward if it's fine to duplicate the nested loop, > but may need to care about removing some useless code. > > > bite the bullet and duplicate that loop nest for the different VMAT_* c= ases. > > Maybe we can merge some of the if (!costing_p) checks by clever > > re-ordering. > > I've tried a bit to merge them if possible, like the place to check > VMAT_CONTIGUOUS, VMAT_CONTIGUOUS_REVERSE and VMAT_CONTIGUOUS_PERMUTE. > But will keep in mind for the following updates. > > > So what > > this series doesn't improve is overall readability of the code (indent = and our > > 80 char line limit). > > Sorry about that. > > > > > The change also makes it more difficult(?) to separate analysis and tra= nsform > > though in the end I hope that analysis will actually "code generate" to= a (SLP) > > data structure so the target will have a chance to see the actual flow = of insns. > > > > That said, I'd like to hear from Richard whether he thinks this is a st= ep > > in the right direction. > > > > Are you willing to followup with doing the same re-structuring to > > vectorizable_store? > > Yes, vectorizable_store was also pointed out in your original suggestion = [1], > I planned to update this once this series meets your expectations and get= s landed. > > > > > OK from my side with the few comments addressed. The patch likely need= s refresh > > after the RVV changes in this area? > > Thanks! Yes, I've updated 2/9 and 3/9 according to your comments, and up= dated > 5/9 and 9/9 as they had some conflicts when rebasing. Re-testing is ongo= ing, > do the updated versions look good to you? Is this series ok for trunk if= all the > test runs go well again as before? Yes. Thanks, Richard. > BR, > Kewen