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

[Andrew Waterman <aswaterman@gmail.com>]

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
> >