linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Jakub Jelinek <jakub@redhat.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	"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, 6 Sep 2019 16:42:58 -0700	[thread overview]
Message-ID: <CAKwvOdk-AQVJnD6-=Z0eUQ6KPvDp2eS2zUV=-oL2K2JBCYaOeQ@mail.gmail.com> (raw)
In-Reply-To: <20190906225606.GF9749@gate.crashing.org>

On Fri, Sep 6, 2019 at 3:56 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Sep 06, 2019 at 03:35:02PM -0700, Nick Desaulniers wrote:
> > On Fri, Sep 6, 2019 at 3:03 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > > And if instead you tested whether the actual feature you need works as
> > > you need it to, it would even work fine if there was a bug we fixed that
> > > breaks things for the kernel.  Without needing a new compiler.
> >
> > That assumes a feature is broken out of the gate and is putting the
> > cart before the horse.  If a feature is available, it should work.
>
> GCC currently has 91696 bug reports.

Fair.

>
> > > Or as another example, if we added support for some other flags. (x86
> > > has only a few flags; many other archs have many more, and in some cases
> > > newer hardware actually has more flags than older).
> >
> > I think compiler flags are orthogonal to GNU C extensions we're discussing here.
>
> No, I am talking exactly about what you brought up.  The flags output
> for inline assembler, using the =@ syntax.  If I had implemented that
> for Power when it first came up, I would by now have had to add support
> for another flag (the CA32 flag).  Oh, and I would not have implemented
> support for OV or SO at all originally, but perhaps they are useful, so
> let's add that as well.  And there is OV32 now as well.

Oh, I misunderstood.  I see your point.  Define the symbol as a number
for what level of output flags you support (ie. the __cplusplus
macro).

>
> > > With the "macro" scheme we would need to add new macros in all these
> > > cases.  And since those are target-specific macros, that quickly expands
> > > beyond reasonable bounds.
> >
> > I don't think so.  Can you show me an example codebase that proves me wrong?
>
> No, of course not, because we aren't silly enough to implement something
> like that.

I still don't think feature detection is worse than version detection
(until you find your feature broken in a way that forces the use of
version detection).

Just to prove my point about version checks being brittle, it looks
like Rasmus' version check isn't even right.  GCC supported `asm
inline` back in the 8.3 release, not 9.1 as in this patch:
https://godbolt.org/z/1woitS .  So users of gcc-8.2 and gcc-8.3
wouldn't be able to take advantage of `asm inline` even though their
compiler supported it.

Or was it "broken" until 9.1?  Lord knows, as `asm inline` wasn't in
any release notes or bug reports I can find:
8: https://gcc.gnu.org/gcc-8/changes.html
9: https://gcc.gnu.org/gcc-9/changes.html
Bug tracker query:
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&cf_known_to_fail_type=allwords&cf_known_to_work_type=allwords&query_format=advanced&short_desc=asm%20inline&short_desc_type=anywordssubstr

Ah, here it is:
https://github.com/gcc-mirror/gcc/commit/6de46ad5326fc5e6b730a2feb8c62d09c1561f92
Which looks like the qualifier was added to this page:
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

I like many of the GNU C extensions, and I want to help support them
in Clang so that they can be used in more places, but the current
process (and questions I have when I implement some of them) leaves me
with the sense that there's probably room for improvement on some of
these things before they go out the door.

Segher, next time there's discussion about new C extensions for the
kernel, can you please include me in the discussions?
-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2019-09-06 23:43 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-29  8:32 [RFC PATCH 0/5] make use of gcc 9's "asm inline()" Rasmus Villemoes
2019-08-29  8:32 ` [RFC PATCH 1/5] treewide: replace __inline__ by inline Rasmus Villemoes
2019-08-29 16:29   ` Joe Perches
2019-08-29  8:32 ` [RFC PATCH 2/5] compiler_types.h: don't #define __inline__ Rasmus Villemoes
2019-08-29  8:32 ` [RFC PATCH 3/5] compiler-gcc.h: add asm_inline definition Rasmus Villemoes
2019-08-29  8:32 ` [RFC PATCH 4/5] x86: alternative.h: use asm_inline for all alternative variants Rasmus Villemoes
2019-08-29  8:32 ` [RFC PATCH 5/5] x86: bug.h: use asm_inline in _BUG_FLAGS definitions Rasmus Villemoes
2019-08-29 16:05 ` [RFC PATCH 0/5] make use of gcc 9's "asm inline()" Linus Torvalds
2019-08-30  7:45   ` Rasmus Villemoes
2019-08-29 17:36 ` Nick Desaulniers
2019-08-29 18:15   ` Linus Torvalds
2019-08-29 18:26     ` Nadav Amit
2019-08-29 18:42     ` Borislav Petkov
2019-08-29 19:41   ` Masahiro Yamada
2019-08-30 23:15 ` [PATCH v2 0/6] " Rasmus Villemoes
2019-08-30 23:15   ` [PATCH v2 1/6] staging: rtl8723bs: replace __inline by inline Rasmus Villemoes
2019-09-04 23:54     ` Nick Desaulniers
2019-08-30 23:15   ` [PATCH v2 2/6] lib/zstd/mem.h: " Rasmus Villemoes
2019-09-04 23:59     ` Nick Desaulniers
2019-09-05  0:07       ` Miguel Ojeda
2019-09-05  9:28         ` Rasmus Villemoes
2019-08-30 23:15   ` [PATCH v2 3/6] compiler_types.h: don't #define __inline Rasmus Villemoes
2019-09-05  0:13     ` Nick Desaulniers
2019-09-05  9:45       ` Rasmus Villemoes
2019-08-30 23:15   ` [PATCH v2 4/6] compiler-gcc.h: add asm_inline definition Rasmus Villemoes
2019-09-05  0:18     ` Nick Desaulniers
2019-09-05  5:43       ` Nadav Amit
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:52           ` Miguel Ojeda
2019-09-05 16:13             ` Miguel Ojeda
2019-09-06 12:23             ` Segher Boessenkool
2019-09-06 15:13               ` Miguel Ojeda
2019-09-06 16:30                 ` Segher Boessenkool
2019-09-06 16:39                   ` Jakub Jelinek
2019-09-06 18:14                     ` Nick Desaulniers
2019-09-06 22:03                       ` Segher Boessenkool
2019-09-06 22:35                         ` Nick Desaulniers
2019-09-06 22:56                           ` Segher Boessenkool
2019-09-06 23:42                             ` Nick Desaulniers [this message]
2019-09-07  0:14                               ` Segher Boessenkool
2019-09-07  1:04                                 ` Nick Desaulniers
2019-09-07 13:11                                   ` Segher Boessenkool
2019-09-08 13:55                                     ` Miguel Ojeda
2019-09-12 21:54                                     ` Nick Desaulniers
2019-09-12 22:12                                       ` Rasmus Villemoes
2019-09-20  0:50                                       ` Segher Boessenkool
2019-09-06 16:47                   ` Miguel Ojeda
2019-08-30 23:15   ` [PATCH v2 5/6] x86: alternative.h: use asm_inline for all alternative variants Rasmus Villemoes
2019-08-30 23:15   ` [PATCH v2 6/6] x86: bug.h: use asm_inline in _BUG_FLAGS definitions Rasmus Villemoes
2019-09-12 22:19   ` [PATCH v3 0/6] make use of gcc 9's "asm inline()" Rasmus Villemoes
2019-09-12 22:19     ` [PATCH v3 1/6] staging: rtl8723bs: replace __inline by inline Rasmus Villemoes
2019-09-29 10:40       ` Greg Kroah-Hartman
2019-09-12 22:19     ` [PATCH v3 2/6] lib/zstd/mem.h: " Rasmus Villemoes
2019-09-12 22:19     ` [PATCH v3 3/6] compiler_types.h: don't #define __inline Rasmus Villemoes
2019-09-12 22:19     ` [PATCH v3 4/6] compiler-types.h: add asm_inline definition Rasmus Villemoes
2019-09-12 22:19     ` [PATCH v3 5/6] x86: alternative.h: use asm_inline for all alternative variants Rasmus Villemoes
2019-09-13  5:41       ` Ingo Molnar
2019-09-12 22:19     ` [PATCH v3 6/6] x86: bug.h: use asm_inline in _BUG_FLAGS definitions Rasmus Villemoes
2019-09-13  5:42       ` Ingo Molnar
2019-09-12 22:30     ` [PATCH v3 0/6] make use of gcc 9's "asm inline()" Miguel Ojeda
2019-09-13  6:11       ` Rasmus Villemoes
2019-09-13 15:21         ` Greg Kroah-Hartman
2019-09-15 18:20           ` 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='CAKwvOdk-AQVJnD6-=Z0eUQ6KPvDp2eS2zUV=-oL2K2JBCYaOeQ@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=miguel.ojeda.sandonis@gmail.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 NNTP newsgroup(s).