From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com [IPv6:2a00:1450:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id 54FCF3858C66 for ; Wed, 26 Jul 2023 08:48:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 54FCF3858C66 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-x22e.google.com with SMTP id 38308e7fff4ca-2b9a28a4166so24096331fa.2 for ; Wed, 26 Jul 2023 01:48:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690361295; x=1690966095; 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=vg19EpTBOQEIMQvBkvW6IM+GHzkjLtXVyl8PUoWOzD0=; b=UYKdpW47FIm7ocfrFwul0d9XBOQoHit+s5r87c8IUnjs5tIyN41BfVoss2H/0/j1jo c2mP3hSh8+bbeaEoewJ5WK9Zwk+Sg2m6qO+1RJu0Y2bSsGHGPPBF0s5V5rieaPT1C24/ YFxg+lMDXh7g0nqPsz0NrJFAOk+fq9+HMTW98p2pBD7hZj++U6OwToTDPyYmYW2NIXwK EPBf8HtnklpC10W2Ss+WWQg3is6QTkcxZ4YA1zmfgwmo8V0r8f+JyDNuePVN7fiilqN7 xeuymHwDFdp7eEHsBPF3ncMvBDZzWYsCHFJ6c3EjfWeUZ7E0o9lk/kqgSyvKOP860WRx sdSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690361295; x=1690966095; 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=vg19EpTBOQEIMQvBkvW6IM+GHzkjLtXVyl8PUoWOzD0=; b=ZllXRIL15p8JBjwcWJkX/jkc7wDDB2he7XNMfffP0QhY4r/0SywPUGhkU4WTERV3rx /GBOjlyGh35rjIjNihmc4kC3f/rh1/NZQum3WWBtwDZvq5kfWdSpfLhaaA1j5HMwimN6 RZLEFhhzctEmjz2O+c0mmw5VD4ENtdKQHCnF5NF/9ugYqblZgYNpR0XvUWYGT0luiEDY uG675X2vrEsSMjOpA8XVyLIMUDogVn2qcZcsV0qhYebmE/4FR/Zs34viBkiSuIUtZ2HQ 00vtJWeqryo8HJ+kJUOztHZeRuq749b5/6urIA44XB5f5Rt6O1D1iPztxmbWcbkkhkX6 WwEQ== X-Gm-Message-State: ABy/qLbg2EZZgjx2fM0utlqlkZKuejHD3MTpKMbaOPIBNqSv6y72bkQI 6aW4pipixKlM3xD+o2N1tMog251CXDWDV3ZYPnA= X-Google-Smtp-Source: APBJJlGpbLd1ffSUf9zPFKR6iGJWQe5/021mVy4ugpTT+XCzjFdALkZp/dp/gcu5GhUvMQ/yl0k/vzFRoySX5DUMyKg= X-Received: by 2002:a2e:8e94:0:b0:2b9:5fd2:763a with SMTP id z20-20020a2e8e94000000b002b95fd2763amr935004ljk.35.1690361295290; Wed, 26 Jul 2023 01:48:15 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Wed, 26 Jul 2023 10:47:29 +0200 Message-ID: Subject: Re: [PATCH] AArch64: Do not increase the vect reduction latency by multiplying count [PR110625] To: Hao Liu OS Cc: Richard Sandiford , "GCC-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,URIBL_BLACK 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, Jul 26, 2023 at 4:02=E2=80=AFAM Hao Liu OS via Gcc-patches wrote: > > > When was STMT_VINFO_REDUC_DEF empty? I just want to make sure that we'= re not papering over an issue elsewhere. > > Yes, I also wonder if this is an issue in vectorizable_reduction. Below = is the the gimple of "gcc.target/aarch64/sve/cost_model_13.c": > > : > # res_18 =3D PHI > # i_20 =3D PHI > _1 =3D (long unsigned int) i_20; > _2 =3D _1 * 2; > _3 =3D x_14(D) + _2; > _4 =3D *_3; > _5 =3D (unsigned short) _4; > res.0_6 =3D (unsigned short) res_18; > _7 =3D _5 + res.0_6; <-- The current stmt_i= nfo > res_15 =3D (short int) _7; > i_16 =3D i_20 + 1; > if (n_11(D) > i_16) > goto ; > else > goto ; > > : > goto ; > > It looks like that STMT_VINFO_REDUC_DEF should be "res_18 =3D PHI "? > The status here is: > STMT_VINFO_REDUC_IDX (stmt_info): 1 > STMT_VINFO_REDUC_TYPE (stmt_info): TREE_CODE_REDUCTION > STMT_VINFO_REDUC_VECTYPE (stmt_info): 0x0 Not all stmts in the SSA cycle forming the reduction have STMT_VINFO_REDUC_DEF set, only the last (latch def) and live stmts have at the moment. Richard. > Thanks, > Hao > > ________________________________________ > From: Richard Sandiford > Sent: Tuesday, July 25, 2023 17:44 > To: Hao Liu OS > Cc: GCC-patches@gcc.gnu.org > Subject: Re: [PATCH] AArch64: Do not increase the vect reduction latency = by multiplying count [PR110625] > > Hao Liu OS writes: > > Hi, > > > > Thanks for the suggestion. I tested it and found a gcc_assert failure: > > gcc.target/aarch64/sve/cost_model_13.c (internal compiler error: in= info_for_reduction, at tree-vect-loop.cc:5473) > > > > It is caused by empty STMT_VINFO_REDUC_DEF. > > When was STMT_VINFO_REDUC_DEF empty? I just want to make sure that > we're not papering over an issue elsewhere. > > Thanks, > Richard > > So, I added an extra check before checking single_defuse_cycle. The upd= ated patch is below. Is it OK for trunk? > > > > --- > > > > The new costs should only count reduction latency by multiplying count = for > > single_defuse_cycle. For other situations, this will increase the redu= ction > > latency a lot and miss vectorization opportunities. > > > > Tested on aarch64-linux-gnu. > > > > gcc/ChangeLog: > > > > PR target/110625 > > * config/aarch64/aarch64.cc (count_ops): Only '* count' for > > single_defuse_cycle while counting reduction_latency. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/aarch64/pr110625_1.c: New testcase. > > * gcc.target/aarch64/pr110625_2.c: New testcase. > > --- > > gcc/config/aarch64/aarch64.cc | 13 ++++-- > > gcc/testsuite/gcc.target/aarch64/pr110625_1.c | 46 +++++++++++++++++++ > > gcc/testsuite/gcc.target/aarch64/pr110625_2.c | 14 ++++++ > > 3 files changed, 69 insertions(+), 4 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/aarch64/pr110625_1.c > > create mode 100644 gcc/testsuite/gcc.target/aarch64/pr110625_2.c > > > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64= .cc > > index 560e5431636..478a4e00110 100644 > > --- a/gcc/config/aarch64/aarch64.cc > > +++ b/gcc/config/aarch64/aarch64.cc > > @@ -16788,10 +16788,15 @@ aarch64_vector_costs::count_ops (unsigned int= count, vect_cost_for_stmt kind, > > { > > unsigned int base > > =3D aarch64_in_loop_reduction_latency (m_vinfo, stmt_info, m_vec_= flags); > > - > > - /* ??? Ideally we'd do COUNT reductions in parallel, but unfortu= nately > > - that's not yet the case. */ > > - ops->reduction_latency =3D MAX (ops->reduction_latency, base * c= ount); > > + if (STMT_VINFO_REDUC_DEF (stmt_info) > > + && STMT_VINFO_FORCE_SINGLE_CYCLE ( > > + info_for_reduction (m_vinfo, stmt_info))) > > + /* ??? Ideally we'd use a tree to reduce the copies down to 1 vec= tor, > > + and then accumulate that, but at the moment the loop-carried > > + dependency includes all copies. */ > > + ops->reduction_latency =3D MAX (ops->reduction_latency, base * co= unt); > > + else > > + ops->reduction_latency =3D MAX (ops->reduction_latency, base); > > } > > > > /* Assume that multiply-adds will become a single operation. */ > > diff --git a/gcc/testsuite/gcc.target/aarch64/pr110625_1.c b/gcc/testsu= ite/gcc.target/aarch64/pr110625_1.c > > new file mode 100644 > > index 00000000000..0965cac33a0 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/pr110625_1.c > > @@ -0,0 +1,46 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-Ofast -mcpu=3Dneoverse-n2 -fdump-tree-vect-details -= fno-tree-slp-vectorize" } */ > > +/* { dg-final { scan-tree-dump-not "reduction latency =3D 8" "vect" } = } */ > > + > > +/* Do not increase the vector body cost due to the incorrect reduction= latency > > + Original vector body cost =3D 51 > > + Scalar issue estimate: > > + ... > > + reduction latency =3D 2 > > + estimated min cycles per iteration =3D 2.000000 > > + estimated cycles per vector iteration (for VF 2) =3D 4.000000 > > + Vector issue estimate: > > + ... > > + reduction latency =3D 8 <-- Too large > > + estimated min cycles per iteration =3D 8.000000 > > + Increasing body cost to 102 because scalar code would issue more q= uickly > > + ... > > + missed: cost model: the vector iteration cost =3D 102 divided by = the scalar iteration cost =3D 44 is greater or equal to the vectorization f= actor =3D 2. > > + missed: not vectorized: vectorization not profitable. */ > > + > > +typedef struct > > +{ > > + unsigned short m1, m2, m3, m4; > > +} the_struct_t; > > +typedef struct > > +{ > > + double m1, m2, m3, m4, m5; > > +} the_struct2_t; > > + > > +double > > +bar (the_struct2_t *); > > + > > +double > > +foo (double *k, unsigned int n, the_struct_t *the_struct) > > +{ > > + unsigned int u; > > + the_struct2_t result; > > + for (u =3D 0; u < n; u++, k--) > > + { > > + result.m1 +=3D (*k) * the_struct[u].m1; > > + result.m2 +=3D (*k) * the_struct[u].m2; > > + result.m3 +=3D (*k) * the_struct[u].m3; > > + result.m4 +=3D (*k) * the_struct[u].m4; > > + } > > + return bar (&result); > > +} > > diff --git a/gcc/testsuite/gcc.target/aarch64/pr110625_2.c b/gcc/testsu= ite/gcc.target/aarch64/pr110625_2.c > > new file mode 100644 > > index 00000000000..7a84aa8355e > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/pr110625_2.c > > @@ -0,0 +1,14 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-Ofast -mcpu=3Dneoverse-n2 -fdump-tree-vect-details -= fno-tree-slp-vectorize" } */ > > +/* { dg-final { scan-tree-dump "reduction latency =3D 8" "vect" } } */ > > + > > +/* The reduction latency should be multiplied by the count for > > + single_defuse_cycle. */ > > + > > +long > > +f (long res, short *ptr1, short *ptr2, int n) > > +{ > > + for (int i =3D 0; i < n; ++i) > > + res +=3D (long) ptr1[i] << ptr2[i]; > > + return res; > > +}