Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause

[Kito Cheng <kito.cheng@sifive.com>]

Hi Andrew:

It's safe to execute on old machine, but it is still a new extension not
included on baseline ISA, so I still prefer having -march to guard that,
and then we can track that in the ELF attribute to see what extensions and
which version are used in the executable / object files.


On Thu, Jan 7, 2021 at 11:51 AM Andrew Waterman <aswaterman@gmail.com>
wrote:

> I've got a contrary opinion:
>
> Since HINTs are guaranteed to execute as no-ops--e.g., this one is
> just a FENCE instruction, which is already a mandatory part of the
> base ISA--they don't _need_ to be called out as separate extensions in
> the toolchain.
>
> Although there's nothing fundamentally wrong with Kito's suggestion,
> it seems like an extra hoop to jump through without commensurate
> benefit.  I see no reason to restrict the use of __builtin_pause,
> since all RISC-V implementations, including old ones, are required to
> support it.  And, of course, that's the reason we encoded it this way
> :)
>
>
> On Wed, Jan 6, 2021 at 7:35 PM Kito Cheng <kito.cheng@gmail.com> wrote:
> >
> > Hi Philipp:
> >
> > Could you add zihintpause to -march parser and guard that on the
> > pattern and builtin like zifencei[1-2]?
> >
> > And could you sent a PR to
> > https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md to
> > mention __builtin_riscv_pause?
> >
> > Thanks!
> >
> > [1] march parser change:
> >
> https://github.com/gcc-mirror/gcc/commit/b03be74bad08c382da47e048007a78fa3fb4ef49
> > [2] Default version for ext.:
> >
> https://github.com/gcc-mirror/gcc/commit/4b81528241ca682025d92558ff6aeec91dafdca8
> >
> >
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
> > > @@ -0,0 +1,10 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2" }  */
> > > +
> > > +void test_pause()
> >
> > I would suggest you change the function name in the testcase,
> > otherwise the scan-assembler test will always pass even if you didn't
> > generate "pause" instruction.
> >
> >
> > > +{
> > > +  __builtin_riscv_pause ();
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler "pause" } } */
> > > +
> > > --
> > > 2.18.4
> > >
>