Re: [PATCH] xen: Use asm inline when available for alternatives

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 22 Apr 2025, Nicola Vetrini wrote:
> On 2025-04-22 22:02, Stefano Stabellini wrote:
> > On Tue, 22 Apr 2025, Nicola Vetrini wrote:
> > > On 2025-04-22 21:58, Stefano Stabellini wrote:
> > > > On Tue, 22 Apr 2025, Andrew Cooper wrote:
> > > > > On 22/04/2025 8:46 pm, Stefano Stabellini wrote:
> > > > > > On Tue, 22 Apr 2025, Andrew Cooper wrote:
> > > > > >> Compilers estimate the size of an asm() block for inlining
> > > purposes.
> > > > > >>
> > > > > >> Constructs such as ALTERNATIVE appear large due to the metadata,
> > > > > depsite often
> > > > > >> only being a handful of instructions.  asm inline() overrides the
> > > > > estimation
> > > > > >> to identify the block as being small.
> > > > > >>
> > > > > >> This has a substantial impact on inlining decisions, expected to be
> > > for
> > > > > the
> > > > > >> better given that the compiler has a more accurate picture to work
> > > > > with.
> > > > > >>
> > > > > >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > > > Hi Andrew,
> > > > > >
> > > > > > If we are going to use asm_inline, please add a note to
> > > > > > docs/misra/C-language-toolchain.rst where we keep record of all the
> > > > > > language extensions we use.
> > > > >
> > > > > It's just asm(), and that's already discussed.
> > > > >
> > > > > I'm not sure what else you think is warranted.
> > > >
> > > >
> > > > If it is just asm() then there is nothing to do. We only need to
> > > > document extensions to the language, nothing else. Many of them are
> > > > already documented under docs/misra/C-language-toolchain.rst (see for
> > > > instance asm and __asm__).
> > > >
> > > > I see the critical part of the patch for this question is:
> > > >
> > > > > +#if CONFIG_CC_HAS_ASM_INLINE
> > > > > +# define asm_inline asm __inline
> > > > > +#else
> > > > > +# define asm_inline asm
> > > > > +#endif
> > > >
> > > > it looks like __inline is an extension ?
> > > 
> > > I see. It was added to the tool configuration, but not there in the rst
> > > (__inline__ as well), hence why there are no CI failures.
> > 
> > I see __inline__|__inline in toolchain.ecl. Nicola, would you be up for
> > sending a small patch to docs/misra/C-language-toolchain.rst to add them?
> 
> Yes, no problem. I'll take the occasion to see whether there are other gaps.

Thanks!


> Perhaps there's a clever way to ensure these files do not go out of sync?

It would take a bit of work, but we could have the list of extensions in
json (like exclude-list.json or safe.json), then we could generate both
C-language-toolchain.rst and toolchain.ecl from the json file.