Re: [PATCH 2/2] asm inline

[Segher Boessenkool <segher@kernel.crashing.org>]

On Sun, Nov 11, 2018 at 04:41:19PM -0700, Martin Sebor wrote:
> On 11/11/2018 03:00 PM, Segher Boessenkool wrote:
> >On Sun, Nov 11, 2018 at 02:33:34PM -0700, Martin Sebor wrote:
> >>On 10/30/2018 11:30 AM, Segher Boessenkool wrote:
> >>>The Linux kernel people want a feature that makes GCC pretend some
> >>>inline assembler code is tiny (while it would think it is huge), so
> >>>that such code will be inlined essentially always instead of
> >>>essentially never.
> >>>
> >>>This patch lets you say "asm inline" instead of just "asm", with the
> >>>result that that inline assembler is always counted as minimum cost
> >>>for inlining.  It implements this for C and C++.
> >>
> >>Rather than overloading the inline keyword I think it would be
> >>more in keeping both with the design of existing features to
> >>control inlining in GCC and with the way the languages are
> >>evolving to allow the always_inline (and perhaps also noinline)
> >>attribute to apply to asm statements.
> >
> >We already "overloaded" the volatile and goto keywords here (it's not
> >overloading, the contexts are completely disjoint).
> >
> >always_inline is a function attribute, and you want to make it a statement
> >attribute (which we do not currently support except for empty statements!)
> >as well.
> 
> Yes, I suggest using a statement attribute for this.  I believe
> it would be more appropriate that overloading the meaning of
> an existing keyword in this context.

It is *not* overloading the meaning in this context.  There *was* no
meaning in this context.

Also, all of the other keywords allowed here (const, volatile, restrict,
goto) have different meanings in other contexts already.

> C++ is entertaining proposals to introduce a few statement
> attributes (e.g., likely/unlikely and even unreachable) so
> as I said, adding one to GCC would be in like with
> the direction the languages are moving (C has adopted the C++
> 11 attributes for C2X and it likely will continue to do so going
> forward).

As I've said many times now, I think attributes are a bad match for this.
Also, I am not going to implement statement attributes just for this.

> Supporting an attribute in this context also has a lower cost
> for third party tools that parse code (and are already prepared
> to deal with attributes).  Adding a new keyword here has a good
> chance of breaking some of those tools.

I don't see why I would care.  Sorry.  Extended asm is a GCC extension,
so we make the rules.

The first patch makes any order and combination of qualifiers (and goto
and inline) valid for asm, where before only zero or one qualifiers and
zero or one goto were allowed (in that order).  This gives exactly the
same problem for 3rd party tools, but it is an obvious improvement (and
even a bugfix).

> >>The inline keyword is commonly understood to be just a hint to
> >>the compiler.
> >
> >That is exactly what it is here.  It hints the compiler that this asm
> >is cheap, whatever it may look like.
> 
> The way you described the purpose of asm inline: "such code will
> be inlined essentially always instead of essentially never"
> sounded to me like it's close to the effect of attribute
> always_inline.  But after reading the patch it looks like the asm
> just overrides the number of instructions GCC estimates
> the statement expands into.

Only for the inlining decisions.  It still uses the conservative
estimate for things like the length attribute (the machine description
thing), and other things required for correctness.

> If I read the code right then
> a different attribute altogether would seem more appropriate
> (e.g., perhaps even something like insn_count (N); that would
> make it possible to provide an exact instruction count and also
> prevent the inlining of the enclosing function by specifying
> a very large count).

No, the user does not know what units is counted in here.  Would there
be any real benefit anyway?  I don't see it.

> In any event, overloading the inline
> keyword for this purpose doesn't seem like a good approach
> to me.

It is not overloading anything.


Segher