public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Nick Desaulniers via gcc-patches" <gcc-patches@gcc.gnu.org>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
		Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
		Rasmus Villemoes <linux@rasmusvillemoes.dk>,
		"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
		"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
		clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH v2 4/6] compiler-gcc.h: add asm_inline definition
Date: Fri, 06 Sep 2019 18:14:00 -0000	[thread overview]
Message-ID: <CAKwvOd=MT_=U250tR+t0jTtj7SxKJjnEZ1FmR3ir_PHjcXFLVw@mail.gmail.com> (raw)
In-Reply-To: <20190906163918.GJ2120@tucnak>

On Fri, Sep 6, 2019 at 9:39 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Sep 06, 2019 at 11:30:28AM -0500, Segher Boessenkool wrote:
> > On Fri, Sep 06, 2019 at 05:13:54PM +0200, Miguel Ojeda wrote:
> > > On Fri, Sep 6, 2019 at 2:23 PM Segher Boessenkool
> > > <segher@kernel.crashing.org> wrote:
> > > > I can't find anything with "feature" and "macros" in the C++ standard,
> > > > it's "predefined macros" there I guess?  In C, it is also "predefined
> > > > macros" in general, and there is "conditional feature macros".
> > >
> > > They are introduced in C++20,
> >
> > (Which isn't the C++ standard yet, okay).
>
> Well, they have been required by SD-6 before being added to C++20, so we
> have tons of the predefined macros for C++ already starting with gcc 4.9 or
> so, but it is something required by the standard so we have to do that.
> Most of them depend also on compiler options, so can't be easily replaced
> with a simple __GNUC__ version check.
>
> What I'd like to add is that each predefined macro isn't without cost,
> while adding one predefined macro might not be measurable (though, for
> some predefined macros (the floating point values) it was very measurable
> and we had to resort to lazy evaluation of the macros), adding hundreds of
> predefined macros is measurable, affects the speed of empty compiler run,
> adds size of -g3 produced debug info, increases size of -E -dD output etc.
>
>         Jakub

Here's the case that I think is perfect:
https://developers.redhat.com/blog/2016/02/25/new-asm-flags-feature-for-x86-in-gcc-6/

Specifically the feature test preprocessor define __GCC_ASM_FLAG_OUTPUTS__.

See exactly how we handle it in the kernel:
- https://github.com/ClangBuiltLinux/linux/blob/0445971000375859008414f87e7c72fa0d809cf8/arch/x86/include/asm/asm.h#L112-L118
- https://github.com/ClangBuiltLinux/linux/blob/0445971000375859008414f87e7c72fa0d809cf8/arch/x86/include/asm/rmwcc.h#L14-L30

Feature detection of the feature makes it trivial to detect when the
feature is supported, rather than brittle compiler version checks.
Had it been a GCC version check, it wouldn't work for clang out of the
box when clang added support for __GCC_ASM_FLAG_OUTPUTS__.  But since
we had the helpful __GCC_ASM_FLAG_OUTPUTS__, and wisely based our use
of the feature on that preprocessor define, the code ***just worked***
for compilers that didn't support the feature ***and*** compilers when
they did support the feature ***without changing any of the source
code*** being compiled.

All I'm asking for is that when GCC adds a new GNU C extension (such
as `asm inline`), define a new preprocessor symbol (like has already
been done w/ __GCC_ASM_FLAG_OUTPUTS__), so that we don't have to use
version checking (or reimplementing autoconf) and use feature
detection instead.  This simplifies use of this feature even between
codebases supporting multiple versions of GCC.

(Also, I'm guessing the cost of another preprocessor define is near
zero compared to parsing comments for -Wimplicit-fallthrough)
-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2019-09-06 18:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190829083233.24162-1-linux@rasmusvillemoes.dk>
     [not found] ` <20190830231527.22304-1-linux@rasmusvillemoes.dk>
     [not found]   ` <20190830231527.22304-5-linux@rasmusvillemoes.dk>
     [not found]     ` <CAKwvOdktYpMH8WnEQwNE2JJdKn4w0CHv3L=YHkqU2JzQ6Qwkew@mail.gmail.com>
2019-09-05 11:07       ` Rasmus Villemoes
2019-09-05 13:45         ` Segher Boessenkool
2019-09-05 14:23           ` Rasmus Villemoes
2019-09-05 14:47             ` Segher Boessenkool
2019-09-05 15:53           ` Miguel Ojeda
2019-09-05 16:13             ` Miguel Ojeda
2019-09-06 12:23             ` Segher Boessenkool
2019-09-06 15:14               ` Miguel Ojeda
2019-09-06 16:30                 ` Segher Boessenkool
2019-09-06 16:39                   ` Jakub Jelinek
2019-09-06 18:14                     ` Nick Desaulniers via gcc-patches [this message]
2019-09-06 22:03                       ` Segher Boessenkool
2019-09-06 22:35                         ` Nick Desaulniers via gcc-patches
2019-09-06 22:56                           ` Segher Boessenkool
2019-09-06 23:43                             ` Nick Desaulniers via gcc-patches
2019-09-07  0:14                               ` Segher Boessenkool
2019-09-07  1:05                                 ` Nick Desaulniers via gcc-patches
2019-09-07 13:11                                   ` Segher Boessenkool
2019-09-08 13:55                                     ` Miguel Ojeda
2019-09-12 21:55                                     ` Nick Desaulniers via gcc-patches
2019-09-12 22:12                                       ` Rasmus Villemoes
2019-09-20  0:50                                       ` Segher Boessenkool
2019-09-06 16:48                   ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKwvOd=MT_=U250tR+t0jTtj7SxKJjnEZ1FmR3ir_PHjcXFLVw@mail.gmail.com' \
    --to=gcc-patches@gcc.gnu.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=segher@kernel.crashing.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).