From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x1132.google.com (mail-yw1-x1132.google.com [IPv6:2607:f8b0:4864:20::1132]) by sourceware.org (Postfix) with ESMTPS id 2A4C83858D1E for ; Wed, 28 Jun 2023 08:20:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2A4C83858D1E 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-yw1-x1132.google.com with SMTP id 00721157ae682-56fff21c2ebso57404027b3.3 for ; Wed, 28 Jun 2023 01:20:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687940433; x=1690532433; 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=mbNUhxKX1JoEkBa2vuBU8icCZn0fS3M18anoX8Sjlvk=; b=X+K/raHU8+LA4t+sKC2EGiF5XGbLWLW2K5i1YCiX2mTo94MDKnonhtxSvuroP4hbdj iujesLvhyTqdMSve3gyPwfwuIsQnG83piJSNupbQr8Fs9HBIrdgXRp6gGghrHAhZ/87x BJGKVLFMAabFRx78pER6Yk1olM6ZaDLB14Mvk5CBwM1blzFFkSeLLpP7y0qZ2r1ituBw RACLfNdz+YGj+1vBc34J9UemqK1HjunW1BLx9S+v/9nlSnMdaAs956mAOp7vF9T+a7MC o3sZUEGmk+PjZ/S21HJG/7uhZqkBPFzjbROP8e1Vy0OfQR7JeY5oZqqpr5xumGiUO1Np cdkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687940433; x=1690532433; 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=mbNUhxKX1JoEkBa2vuBU8icCZn0fS3M18anoX8Sjlvk=; b=LZoNu7PeU4uxU558zrHs8BPNiWlqJWObOBPP8IvJXSJCcqcqj8yVwcZgz4R1Xgowng aYzgAOOn1LTFsERGOdAEs7ll2nMOCmWTs7WSscdmKWepfL8VTIprKjQJ0rRRori1vqzG v58Bra2aC1WNr3RB+QfFFqfSGQCG5dGcvACcLKumZJdhXpZ8hzlNDZLGBO51wLJbNkuv CH0fPz5r4R10cqNH6nq/wRCx1AfMzIvlwSr28Ww0PPLtAAn9mJCoeZZfYIXpifw+htfl linVY91JWCXCLKEJD6Xl5Im3jatPZ2AMW5qcaXv0z8Ln/ocVtFXGOVH9CO1JpiA4lDEK 0Rng== X-Gm-Message-State: AC+VfDyntt+TEVNtFNnQG1wn+oxKf2OvaI7joJ7evSxfwAgQpytg9vcf LUTaZlUXbEJzMfS66HrncQxAZ1iJEnqiEWPkvrM= X-Google-Smtp-Source: ACHHUZ5MVc51J0txqTOxA/b7s+E+UY2mVPrqyA0NLKlroBMu/ceL6pLKcEnEroBekEtJmfK6LNcuEM653vMXjkPlvEk= X-Received: by 2002:a81:6ac6:0:b0:568:b6a5:9100 with SMTP id f189-20020a816ac6000000b00568b6a59100mr36557790ywc.42.1687940433197; Wed, 28 Jun 2023 01:20:33 -0700 (PDT) MIME-Version: 1.0 References: <20230626023408.33758-1-hongyu.wang@intel.com> In-Reply-To: From: Hongyu Wang Date: Wed, 28 Jun 2023 16:13:15 +0800 Message-ID: Subject: Re: [PATCH] i386: Relax inline requirement for functions with different target attrs To: Uros Bizjak Cc: Hongyu Wang , gcc-patches@gcc.gnu.org, hongtao.liu@intel.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,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: > If the user specified a different arch for callee than the caller, > then the compiler will switch on different ISAs (-march is just a > shortcut for different ISA packs), and the programmer is aware that > inlining isn't intended here (we have -mtune, which is not as strong > as -march, but even functions with different -mtune are not inlined > without always_inline attribute). This is documented as: The original issue comes from a case like float callee (float a, float b, float c, float d, float e, float f, float g, float h) { return a * b + c * d + e * f + g + h + a * c + b * c + a * d + b * e + a * f + c * h + b * (a - 0.4f) * (c + h) * (b + e * d) - a / f * h; } __attribute__((target_clones("default","arch=3Dicelake-server"))) void caller (int n, float *a, float c1, float c2, float c3, float c4, float c5, float c6, float c7) { for (int i =3D 0; i < n; i++) { a[i] =3D callee (a[i], c1, c2, c3, c4, c5, c6, c7); } } For current gcc, the .icelake_server clone fails to inline callee due to target specific option mismatch, while the .default clone succeeded and the loop get vectorized. I think it is not reasonable that the specific clone with higher arch cannot produce better code. So I think at least we can decide to inline those callee without any arch/tune specified, but for now they are rejected by the strict arch=3D and tune=3D check. Uros Bizjak =E4=BA=8E2023=E5=B9=B46=E6=9C=8828=E6=97=A5= =E5=91=A8=E4=B8=89 14:43=E5=86=99=E9=81=93=EF=BC=9A > > On Wed, Jun 28, 2023 at 3:56=E2=80=AFAM Hongyu Wang wrote: > > > > > I don't think this is desirable. If we inline something with differen= t > > > ISAs, we get some strange mix of ISAs when the function is inlined. > > > OTOH - we already inline with mismatched tune flags if the function i= s > > > marked with always_inline. > > > > Previously ix86_can_inline_p has > > > > if (((caller_opts->x_ix86_isa_flags & callee_opts->x_ix86_isa_flags) > > !=3D callee_opts->x_ix86_isa_flags) > > || ((caller_opts->x_ix86_isa_flags2 & callee_opts->x_ix86_isa_flags= 2) > > !=3D callee_opts->x_ix86_isa_flags2)) > > ret =3D false; > > > > It make sure caller ISA is a super set of callee, and the inlined one > > should follow caller's ISA specification. > > > > IMHO I cannot give a real example that after inline the caller's > > performance get harmed, I added PVW since there might > > be some callee want to limit its vector size and caller may have > > larger preferred vector size. At least with current change > > we get more optimization opportunity for different target_clones. > > > > But I agree the tuning setting may be a factor that affect the > > performance. One possible choice is that if the > > tune for callee is unspecified or default, just inline it to the > > caller with specified arch and tune. > > If the user specified a different arch for callee than the caller, > then the compiler will switch on different ISAs (-march is just a > shortcut for different ISA packs), and the programmer is aware that > inlining isn't intended here (we have -mtune, which is not as strong > as -march, but even functions with different -mtune are not inlined > without always_inline attribute). This is documented as: > > --q-- > On the x86, the inliner does not inline a function that has different > target options than the caller, unless the callee has a subset of the > target options of the caller. For example a function declared with > target("sse3") can inline a function with target("sse2"), since -msse3 > implies -msse2. > --/q-- > > I don't think arch=3Dskylake can be considered as a subset of arch=3Dicel= ake-server. > > I agree that the compiler should reject functions with different PVW. > This is also in accordance with the documentation. > > Uros. > > > > > Uros Bizjak via Gcc-patches =E4=BA=8E2023=E5= =B9=B46=E6=9C=8827=E6=97=A5=E5=91=A8=E4=BA=8C 17:16=E5=86=99=E9=81=93=EF=BC= =9A > > > > > > > > > > > > On Mon, Jun 26, 2023 at 4:36=E2=80=AFAM Hongyu Wang wrote: > > > > > > > > Hi, > > > > > > > > For function with different target attributes, current logic reject= s to > > > > inline the callee when any arch or tune is mismatched. Relax the > > > > condition to honor just prefer_vecotr_width_type and other flags th= at > > > > may cause safety issue so caller can get more optimization opportun= ity. > > > > > > I don't think this is desirable. If we inline something with differen= t > > > ISAs, we get some strange mix of ISAs when the function is inlined. > > > OTOH - we already inline with mismatched tune flags if the function i= s > > > marked with always_inline. > > > > > > Uros. > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,} > > > > > > > > Ok for trunk? > > > > > > > > gcc/ChangeLog: > > > > > > > > * config/i386/i386.cc (ix86_can_inline_p): Do not check arc= h or > > > > tune directly, just check prefer_vector_width_type and make= sure > > > > not to inline if they mismatch. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.target/i386/inline-target-attr.c: New test. > > > > --- > > > > gcc/config/i386/i386.cc | 11 +++++---- > > > > .../gcc.target/i386/inline-target-attr.c | 24 +++++++++++++++= ++++ > > > > 2 files changed, 30 insertions(+), 5 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.target/i386/inline-target-att= r.c > > > > > > > > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > > > > index 0761965344b..1d86384ac06 100644 > > > > --- a/gcc/config/i386/i386.cc > > > > +++ b/gcc/config/i386/i386.cc > > > > @@ -605,11 +605,12 @@ ix86_can_inline_p (tree caller, tree callee) > > > > !=3D (callee_opts->x_target_flags & ~always_inline_s= afe_mask)) > > > > ret =3D false; > > > > > > > > - /* See if arch, tune, etc. are the same. */ > > > > - else if (caller_opts->arch !=3D callee_opts->arch) > > > > - ret =3D false; > > > > - > > > > - else if (!always_inline && caller_opts->tune !=3D callee_opts->t= une) > > > > + /* Do not inline when specified perfer-vector-width mismatched b= etween > > > > + callee and caller. */ > > > > + else if ((callee_opts->x_prefer_vector_width_type !=3D PVW_NONE > > > > + && caller_opts->x_prefer_vector_width_type !=3D PVW_NONE= ) > > > > + && callee_opts->x_prefer_vector_width_type > > > > + !=3D caller_opts->x_prefer_vector_width_type) > > > > ret =3D false; > > > > > > > > else if (caller_opts->x_ix86_fpmath !=3D callee_opts->x_ix86_fpm= ath > > > > diff --git a/gcc/testsuite/gcc.target/i386/inline-target-attr.c b/g= cc/testsuite/gcc.target/i386/inline-target-attr.c > > > > new file mode 100644 > > > > index 00000000000..995502165f0 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.target/i386/inline-target-attr.c > > > > @@ -0,0 +1,24 @@ > > > > +/* { dg-do compile } */ > > > > +/* { dg-options "-O2" } */ > > > > +/* { dg-final { scan-assembler-not "call\[ \t\]callee" } } */ > > > > + > > > > +__attribute__((target("arch=3Dskylake"))) > > > > +int callee (int n) > > > > +{ > > > > + int sum =3D 0; > > > > + for (int i =3D 0; i < n; i++) > > > > + { > > > > + if (i % 2 =3D=3D 0) > > > > + sum +=3Di; > > > > + else > > > > + sum +=3D (i - 1); > > > > + } > > > > + return sum + n; > > > > +} > > > > + > > > > +__attribute__((target("arch=3Dicelake-server"))) > > > > +int caller (int n) > > > > +{ > > > > + return callee (n) + n; > > > > +} > > > > + > > > > -- > > > > 2.31.1 > > > >