Re: [PATCH 20/22] x86,word-at-a-time: Remove .fixup usage

[Nick Desaulniers <ndesaulniers@google.com>]

On Tue, Nov 9, 2021 at 12:23 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Nov 08, 2021 at 10:53:31AM -0800, Nick Desaulniers wrote:
> > On Mon, Nov 8, 2021 at 10:29 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Mon, Nov 08, 2021 at 08:47:11AM -0800, Josh Poimboeuf wrote:
> > > > On Fri, Nov 05, 2021 at 06:10:43PM +0100, Peter Zijlstra wrote:
> > > > > +static inline unsigned long load_unaligned_zeropad(const void *addr)
> > > > > +{
> > > > > +   unsigned long offset, data;
> > > > > +   unsigned long ret;
> > > > > +
> > > > > +   asm_volatile_goto(
> > > > > +           "1:     mov %[mem], %[ret]\n"
> > > > > +
> > > > > +           _ASM_EXTABLE(1b, %l[do_exception])
> > > > > +
> > > > > +           : [ret] "=&r" (ret)
> > > > > +           : [mem] "m" (*(unsigned long *)addr)
> > > > > +           : : do_exception);
> > > > > +
> > > > > +out:
> > > > > +   return ret;
> > > > > +
> > > > > +do_exception: __cold;
> > > >
> > > > Clang doesn't approve of this label annotation:
> > > >
> > > > In file included from fs/dcache.c:186:
> > > > ./arch/x86/include/asm/word-at-a-time.h:99:15: warning: '__cold__' attribute only applies to functions [-Wignored-attributes]
> > > > do_exception: __cold;
> > >
> > > /me mutters something best left unsaid these days...
> > >
> > > Nick, how come?
> >
> > Looks like https://bugs.llvm.org/show_bug.cgi?id=47487.
>
> Indeed it does. So what do we do? Keep the attribute and ignore the warn
> on clang for now? Even if techinically useless, I do like it's
> descriptive nature.

I think the feature of label-attributes is generally useful, for asm
goto (without outputs) and probably computed-goto, so I think we
should implement support for these in clang.  I suspect the machinery
for hot/cold labels was added to clang and LLVM before asm goto was;
LLVM likely has all the machinery needed and we probably just need to
relax or adjust clang's semantic analysis of where the attribute may
occur.

With the above patch, we'd still have issues though with released
versions of clang, and -Werror would complicate things further.

I think the use of this feature (label-attributes) here isn't
necessary though; because of the use of outputs, the "fallthrough"
basic block needs to be placed immediately after the basic block
terminated by the asm goto, at least in LLVM.  Was different ordering
of basic blocks observed with GCC without this label attribute?

_Without_ outputs, I can see being able to specify which target of an
asm-goto with multiple labels is relatively hot as useful, but _with_
outputs I suspect specifying the indirect targets as cold provides
little to no utility.  Unless the cold attribute is helping move
("shrink-wrap"?) the basic block to a whole other section
(.text.cold.)?
-- 
Thanks,
~Nick Desaulniers