Hi!
On Tue, Dec 04, 2018 at 03:30:47PM +0000, Richard Sandiford wrote:
> Segher Boessenkool <seg...@kernel.crashing.org> writes:
> Hmm, so we allow top-level "asm volatile" in C++ but not C?
Apparently. Evert top-level asm is effectively volatile, so this doesn't
really matter; but should we try to resolve the difference?
> Probably worth having tests to show that we (intentionally) don't
> allow top-level "asm inline".
Okay, I'll do this separately.
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -8382,6 +8382,10 @@ various @option{-std} options, use @code{__asm__}
> > instead of
> > @item volatile
> > The optional @code{volatile} qualifier has no effect.
> > All basic @code{asm} blocks are implicitly volatile.
> > +
> > +@item inline
> > +If you use the @code{inline} qualifier, then for inlining purposes the size
> > +of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
> > @end table
> >
> > @subsubheading Parameters
>
> You need to update the syntax above too, which currently reads:
>
> @example
> asm @r{[} volatile @r{]} ( @var{AssemblerInstructions} )
> @end example
That was modified in patch 1?
@example
asm @var{asm-qualifiers} ( @var{AssemblerInstructions} )
@end example
> > +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline. */
> > +
> > +static inline bool
> > +gimple_asm_inline_p (const gasm *asm_stmt)
> > +{
> > + return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> > +}
>
> Return true if ASM_STMT is ...
Heh, I copied this from the "volatile" one above. I'll fix that one, too.
> (Or "Return true if asm statement ASM_STMT is marked inline", since gasm
> forces it to be an asm statement.)
Yup, same deal.
> > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> > index 5aa782b..7e9ed99 100644
> > --- a/gcc/tree-inline.c
> > +++ b/gcc/tree-inline.c
> > @@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights
> > *weights)
> > with very long asm statements. */
> > if (count > 1000)
> > count = 1000;
> > + /* If this asm is asm inline, count anything as minimum size. */
> > + if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> > + count = !!count;
>
> FWIW, since Marc found it confusing too... I think MIN (count, 1) would
> show the intent more clearerly. But that's just personal preference,
> no need to change.
MIN is a nice way to say it, thanks! Better than "!!" or "!= 0" here.
Thanks for the review, I'm committing it with the changes above (later
today, after a final bootstrap),
Segher
