From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id AD74A3892440 for ; Thu, 22 Apr 2021 01:01:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AD74A3892440 Received: by mail-ot1-x336.google.com with SMTP id g4-20020a9d6b040000b029029debbbb3ecso7671335otp.7 for ; Wed, 21 Apr 2021 18:01:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tQxvAoh+iZBiHBRPQiWPaGBsm+9lVq6zkMBEaB0g0Xw=; b=DBNRwo0jjsGQ6Qzd+30jDGQbTkthtLKRKo0I81DKUasMTVXW0j9xBc+QimoVRXZ+3i /WLvLG9SGu7BBaonmYl/lpSAklN5ANt7qMrO5axm6LI/jHt17d34PZId+jCsdzzLAbS7 gCgGXBMKegIHj7hJMArrzdms5OCLfWQh7e7fsixuUJo+4k/CGctWzW4/RiLHlRP6Be1Z I6ZQT6cFmSHh+O8R9wW1Vd4o0GCA2MFdsEE31o+3zxTzqapn4NMoJB/QmPnjp3m2COoQ 9b/lbVTRjajjy2RzQwAeF49IwRAZOYYHpPCPd7Uh70XZX5zozxfoeWCa2tXrt7V0HXeD 7O+g== X-Gm-Message-State: AOAM5333nQEwcHuGq8NAXHIdvpHbj0vuur7EPlmElJ/IIm5T0SM8Zf/B 9YhugbcR1iNwKu9DHbxz0sJw0zbpXh/AAJPQwmw= X-Google-Smtp-Source: ABdhPJzbdiFPbpvmM8yS8dOOUL7mUi3jbqw7ng0sNvwW1qc67ZsRArYxuD5WdeCS0DwSJiewa6kCoqPGYnuJJswXJsc= X-Received: by 2002:a9d:12e:: with SMTP id 43mr718234otu.90.1619053304033; Wed, 21 Apr 2021 18:01:44 -0700 (PDT) MIME-Version: 1.0 References: <20210414223918.230495-1-hjl.tools@gmail.com> <20210414223918.230495-3-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Wed, 21 Apr 2021 18:01:07 -0700 Message-ID: Subject: Re: [PATCH v4 2/2] x86: Add general_regs_only function attribute To: Martin Sebor Cc: gcc-patches@gcc.gnu.org, Uros Bizjak , Jakub Jelinek , Bernhard Reutner-Fischer , Richard Biener Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3029.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Apr 2021 01:01:46 -0000 On Wed, Apr 21, 2021 at 4:24 PM Martin Sebor wrote: > > On 4/21/21 2:58 PM, H.J. Lu wrote: > > On Wed, Apr 21, 2021 at 10:09 AM Martin Sebor wrote: > >> > >> On 4/14/21 4:39 PM, H.J. Lu wrote: > >>> commit 87c753ac241f25d222d46ba1ac66ceba89d6a200 > >>> Author: H.J. Lu > >>> Date: Fri Aug 21 09:42:49 2020 -0700 > >>> > >>> x86: Add target("general-regs-only") function attribute > >>> > >>> is incomplete since it is impossible to call integer intrinsics from > >>> a function with general-regs-only target attribute. > >>> > >>> 1. Add general_regs_only function attribute to inform the compiler that > >>> functions use only general purpose registers. When making inlining > >>> decisions on such functions, non-GPR compiler options are excluded. > >>> 2. Add general_regs_only attribute to x86 intrinsics which use only > >>> general purpose registers. > >>> > >> ... > >>> --- a/gcc/doc/extend.texi > >>> +++ b/gcc/doc/extend.texi > >>> @@ -7066,6 +7066,11 @@ On x86 targets, the @code{fentry_section} attribute sets the name > >>> of the section to record function entry instrumentation calls in when > >>> enabled with @option{-pg -mrecord-mcount} > >>> > >>> +@item general_regs_only > >>> +@cindex @code{general_regs_only} function attribute, x86 > >>> +The @code{general_regs_only} attribute on functions is used to > >>> +inform the compiler that functions use only general purpose registers. > >> > >> I'll just reiterate basically the same comment as before: it's not > >> clear from the very brief description above what the requirements > >> are for using the attribute. I'm guessing it can be applied to > >> any function (inline or otherwise) but only has any effect when > >> the function is actually inlined and otherwise doesn't constrain > >> what the function can do. (Whatever the constraints are, I think > >> the manual should spell them out, and likewise for its effects.) > > > > That is correct. > > > >> Similarly it's not clear what should be expected when the function > >> does use some other register. Ideally, I think GCC would check and > >> issue a nice error message whether or not the function is inlined > >> or called. I suspect that might only be possible for inline > >> functions that are actually called and for which the back end must > >> emit code. > > > > This is what GCC does today: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99744 > > Yes, that's the rather obscure error I think I commented on before > and suggested should be improved. Based on r99744-3.c I don't think > this has changed in the improved patch. My goal is to fix the inline failures, not to improve the compiler error message. > > > >> Other than that, I'd suggest to improve the phrasing a bit: > >> > >> The @code{general_regs_only} function attribute indicates that > >> the function uses only general purpose registers... [text > >> explaining constraints and errors follows]. > >> > >> Martin > > > > How about this > > > > @item general_regs_only > > @cindex @code{general_regs_only} function attribute, x86 > > The @code{general_regs_only} attribute on functions is used to inform > > the compiler that functions use only general purpose registers. It > > can be used together with the @code{always_inline} attribute to avoid > > inlining failure when there is a mismatch in compiler vector options. > > Without an article the part "that functions use only general purpose > registers" is unclear and/or grammatically incorrect. What functions? > If the function the attribute is applied to, it needs an article, e.g., > "the function" or "a function", and singular. (Otherwise it could be > read as talking about the functions called from the one with > the attribute, or some other functions altogether). > > I tried to correct that above but, if you prefer, the following would > be closer to your phrasing but more correct/accurate: > > The @code{general_regs_only} function attribute informs > the compiler that the function uses only general purpose > registers. > > I don't understand what the second sentence is trying to say, and > without a better error message for the problem in r99744, I suspect > few users will either. I am suggesting to explain in the text you > are adding, under what conditions inlining might fail without > the attribute, and what effect the attribute has on the function > that prevents the inlining failure. How about this? @item general_regs_only @cindex @code{general_regs_only} function attribute, x86 The @code{general_regs_only} function attribute informs the compiler that the function uses only general purpose registers. When the compiler inlines a function with the @code{always_inline} attribute, target-specific compilation options may lead to inline failures. The @code{general_regs_only} attribute, if applicable, can be used together with the @code{always_inline} attribute to reduce inlining failure. > (If we can't explain what the effect is then I wonder why > the attribute is being added at all instead of teaching GCC to > always behave as if the attribute were there when its absence > would otherwise lead to an error.) > Inlining an always_inline function into a function, which doesn't support the ISA needed by the always_inline function, should fail. But when inlining the always_inline function, the compiler doesn't know if GPR instructions are sufficient for the always_inline function. The general_regs_only informs the compiler that the function uses only general purpose registers. There are no other ways for the compiler to deduce such info at this stage. -- H.J.