From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by sourceware.org (Postfix) with ESMTPS id 4C9153858410 for ; Wed, 28 Jun 2023 06:43:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4C9153858410 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-ed1-x533.google.com with SMTP id 4fb4d7f45d1cf-5149aafef44so5930672a12.0 for ; Tue, 27 Jun 2023 23:43:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687934587; x=1690526587; 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=99xSvmZwHrwZ1cG6SyYKrkPv1g2tbNGaAKx2raukmJk=; b=m0FCykSfJoCRqmh3x/1X5SZHWcxz9sJE8n9ZLYsrxFBlB6bbyuvGXj/2RORK3CV+y6 FI4QRD3yzMzv7h4Llzh2AcVksHG36Sw6YhhaUpX1bKj31jIXniq6bMtfSWFtBr1xDxRJ /4NVZ1hu7N9LQ46sYyiav/DBn985mcZs4kNea6AHTfHMb9XbGP55V0p3qwaIfpjR09oU Rw2wzdhnQSlFxzdOVvpjua5nBxwxfuHSlIiCz2nSFHg6tt0ZUOVx9BlAgvFx/Kdpb0Vo BZDLFb972dgHT4OtAiYC6Bfsm9b5ESMMp1PjrVqMLBYp8L4zQR0AxqcW8dfGbNUOWtr+ oGZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687934587; x=1690526587; 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=99xSvmZwHrwZ1cG6SyYKrkPv1g2tbNGaAKx2raukmJk=; b=jLYU5GHX/zHut92Qqkan5pgfW5ed1PikyUdn2eEWkeKlk++ulwYtJuze52S2crQIq9 Bp+nxCI3CVszGt9KxO9v0K/skRKd4soIJ0Iu5jEJGRzSLsT98hd5sNGTI7DHMRxsFv+J OwWW7q8C4QEiJ8dp1HiI7uqTvTk1kEtFPw48YcvHBINPBs5Ny7sHh2aXPVfsWUR+mzJ4 2xaWAZcJCsANBnErqN0V9GZD4IwmsKvjUIeuAw43YRvxTXexy2jJcmy2UfEJXN3ujO7i 00t5Q3sYh3G1ZlUE8k2XskWt8vLaAhCRv10Vo3KW20A3cGRJxmV5L/Dw9xIWfTJgyYRK gVQw== X-Gm-Message-State: AC+VfDzYiGH9Nv9WMuGzOZTkUlg8/HYNMzaC/KjALGI4YvQeKxvDLMWb 7DEaZkQI6NLPt8HjKTrYzq5AxS3otjuU5eeDcu0r1UYOWJblRQ== X-Google-Smtp-Source: ACHHUZ7L5dOVzNsa3tBOu+kzhAm2WVbg3vqkMvXsNCP4RKov5DT3sIlaEftkRxH6/vuA+rTdA9l5ZqFlMtjfkf3BEt4= X-Received: by 2002:aa7:d94c:0:b0:51d:8056:8a2f with SMTP id l12-20020aa7d94c000000b0051d80568a2fmr8343551eds.24.1687934586750; Tue, 27 Jun 2023 23:43:06 -0700 (PDT) MIME-Version: 1.0 References: <20230626023408.33758-1-hongyu.wang@intel.com> In-Reply-To: From: Uros Bizjak Date: Wed, 28 Jun 2023 08:42:55 +0200 Message-ID: Subject: Re: [PATCH] i386: Relax inline requirement for functions with different target attrs To: Hongyu Wang 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.8 required=5.0 tests=BAYES_00,BODY_8BITS,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 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 28, 2023 at 3:56=E2=80=AFAM Hongyu Wang wrote: > > > I don't think this is desirable. If we inline something with different > > ISAs, we get some strange mix of ISAs when the function is inlined. > > OTOH - we already inline with mismatched tune flags if the function is > > 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_flags2) > !=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=3Dicelak= e-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 rejects = to > > > inline the callee when any arch or tune is mismatched. Relax the > > > condition to honor just prefer_vecotr_width_type and other flags that > > > may cause safety issue so caller can get more optimization opportunit= y. > > > > I don't think this is desirable. If we inline something with different > > ISAs, we get some strange mix of ISAs when the function is inlined. > > OTOH - we already inline with mismatched tune flags if the function is > > 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 arch = or > > > tune directly, just check prefer_vector_width_type and make s= ure > > > 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-attr.= 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_saf= e_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->tun= e) > > > + /* Do not inline when specified perfer-vector-width mismatched bet= ween > > > + 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_fpmat= h > > > diff --git a/gcc/testsuite/gcc.target/i386/inline-target-attr.c b/gcc= /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 > > >