Committed with typo fix (on the title) and ChangeLog fix.

On Thu, Jul 9, 2020 at 2:48 PM Kito Cheng <kito.ch...@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 4:03 AM Jim Wilson <j...@sifive.com> wrote:
> >
> > On Tue, Jul 7, 2020 at 2:52 AM Kito Cheng <kito.ch...@sifive.com> wrote:
> > > gcc/ChangeLog:
> > >         * gcc/config/riscv/riscv.md (): New.
> > >         (TP_REGNUM): Ditto.
> > >         * doc/extend.texi (Target Builtins): Add RISC-V built-in section.
> > >         Document __builtin_thread_pointer.
> > > gcc/testsuite/ChangeLog:
> > >         * gcc.target/riscv/read-thread-pointer.c: New.
> >
> > It looks OK to me in general.
> >
> > You added builtin_thread_pointer but not builtin_set_thread_pointer.
> > Maybe we should implement both as long as we are implementing one?  If
> > clang only implements one, maybe it should implement the other also?
> > This doesn't have to be part of this patch.  This could be a separate
> > issue.
>
> LLVM side only implements builtin_thread_pointer at this moment,
> and I also saw the gcc and only half target are implemented both,
> so I only implement builtin_thread_pointer first, but I think it's good idea
> to implement both.
>
> > The builtin_thread_pointer docs looks out-of-date.  It is documented
> > for alpha and SH, but it is implemented in gcc/builtins.c not in the
> > backends.  A scan of md files show that quite a few targets support it
> > but don't document it.  I think it should be documented in the generic
> > builtins section not in the target dependent builtins sections with
> > some language that says not all targets support it.  This doesn't have
> > to be part of this patch.  This could be a separate issue.
>
> Yeah, I've found the issue when I seeking the writing material for the doc :P
>
>
> > We have two existing undocumented builtins.  __builtin_riscv_fsflags
> > and __builtin_riscv_frflags for setting or reading the FP flags.  I
> > don't know if anyone uses them though.  newlib and glbic both use
> > extended asms for these operations.  This doesn't have to be part of
> > this patch.  This could be a separate issue.
>
> Got it.
>
> > There is a document https://github.com/riscv/riscv-c-api-doc for
> > coordinating gcc and llvm work that has an empty list of builtin
> > functions.  I'm not sure if this document is still useful.  If this is
> > a RISC-V specific builtin then it should be listed here, but I don't
> > think it should be considered a RISC-V specific builtin.  There is an
> > unresolved pull request for the frflags and fsflags builtins.  I guess
> > I forgot about that.
>
> Oh...I even forgot the PR was opened by me...
>
> >
> > Jim

Reply via email to