[COMMITTED 1/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[COMMITTED 1/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[Palmer Dabbelt]

From: Hsiangkai Wang <kai.wang@sifive.com>

In some cases, we do not want to go through the resolver for function
calls. For example, functions with vector arguments will use vector
registers to pass arguments. In the resolver, we do not save/restore the
vector argument registers for lazy binding efficiency. To avoid ruining
the vector arguments, functions with vector arguments will not go
through the resolver.

To achieve the goal, we will annotate the function symbols with
STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic
section. In the first pass on PLT relocations, we do not set up to call
_dl_runtime_resolve. Instead, we resolve the functions directly.

Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com>
Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Link: https://inbox.sourceware.org/libc-alpha/20230314162512.35802-1-kito.cheng@sifive.com
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 elf/elf.h                    |  7 +++++++
 manual/platform.texi         |  6 ++++++
 sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++
 sysdeps/riscv/dl-machine.h   | 26 ++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)
 create mode 100644 sysdeps/riscv/dl-dtprocnum.h

diff --git a/elf/elf.h b/elf/elf.h
index 94ca23c1bb..4f65b5a32d 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -3933,6 +3933,13 @@ enum
 
 #define R_TILEGX_NUM		130
 
+/* RISC-V specific values for the Dyn d_tag field.  */
+#define DT_RISCV_VARIANT_CC	(DT_LOPROC + 1)
+#define DT_RISCV_NUM		2
+
+/* RISC-V specific values for the st_other field.  */
+#define STO_RISCV_VARIANT_CC 0x80
+
 /* RISC-V ELF Flags */
 #define EF_RISCV_RVC 			0x0001
 #define EF_RISCV_FLOAT_ABI 		0x0006
diff --git a/manual/platform.texi b/manual/platform.texi
index c6ed73cb97..714c07813f 100644
--- a/manual/platform.texi
+++ b/manual/platform.texi
@@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium.
 @node RISC-V
 @appendixsec RISC-V-specific Facilities
 
+Functions that are lazily bound must be compatible with the standard calling
+convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means
+this function is not compatible with the standard calling convention. The
+dynamic linker will directly resolve it instead of using the lazy binding
+mechanism.
+
 Cache management facilities specific to RISC-V systems that implement the Linux
 ABI are declared in @file{sys/cachectl.h}.
 
diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h
new file mode 100644
index 0000000000..281c5aadeb
--- /dev/null
+++ b/sysdeps/riscv/dl-dtprocnum.h
@@ -0,0 +1,21 @@
+/* Configuration of lookup functions.  RISC-V version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* Number of extra dynamic section entries for this architecture.  By
+   default there are none.  */
+#define DT_THISPROCNUM	DT_RISCV_NUM
diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
index c0c9bd93ad..0e6c0bb155 100644
--- a/sysdeps/riscv/dl-machine.h
+++ b/sysdeps/riscv/dl-machine.h
@@ -53,6 +53,9 @@
      || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64)))	\
    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
 
+//* Translate a processor specific dynamic tag to the index in l_info array.  */
+#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM)
+
 /* Return nonzero iff ELF header is compatible with the running host.  */
 static inline int __attribute_used__
 elf_machine_matches_host (const ElfW(Ehdr) *ehdr)
@@ -281,6 +284,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
   /* Check for unexpected PLT reloc type.  */
   if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT))
     {
+      if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL))
+	{
+          /* Check the symbol table for variant CC symbols.  */
+          const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info);
+          const ElfW(Sym) *symtab =
+            (const void *)D_PTR (map, l_info[DT_SYMTAB]);
+          const ElfW(Sym) *sym = &symtab[symndx];
+          if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC))
+            {
+              /* Avoid lazy resolution of variant CC symbols.  */
+              const struct r_found_version *version = NULL;
+              if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
+                {
+                  const ElfW(Half) *vernum =
+                    (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]);
+                  version = &map->l_versions[vernum[symndx] & 0x7fff];
+                }
+              elf_machine_rela (map, scope, reloc, sym, version, reloc_addr,
+                                skip_ifunc);
+              return;
+            }
+	}
+
       if (__glibc_unlikely (map->l_mach.plt == 0))
 	{
 	  if (l_addr)
-- 
2.40.0

[COMMITTED 2/2] Remap __GLIBC_FLT_EVAL_METHOD to 0 if __FLT_EVAL_METHOD__ is -1

[Palmer Dabbelt]

From: Kito Cheng <kito.cheng@sifive.com>

__GLIBC_FLT_EVAL_METHOD will effect the definition of float_t and
double_t, currently we'll set __GLIBC_FLT_EVAL_METHOD to 2 when
__FLT_EVAL_METHOD__ is -1, that means we'll define float_t and double_t
to long double.

However some target isn't natively (HW) support long double like AArch64 and
RISC-V, they defined long double as 128-bits IEEE 754 floating point type.

That means setting __GLIBC_FLT_EVAL_METHOD to 2 will cause very inefficient
code gen for those target who didn't provide native support for long
double, and that's violate the spirit float_t and double_t - most efficient
types at least as wide as float and double.

So this patch propose to remap __GLIBC_FLT_EVAL_METHOD to 0 rather than
2 when __FLT_EVAL_METHOD__ is -1, which means we'll use float/double
rather than long double for float_t and double_t.

Note: __FLT_EVAL_METHOD__ == -1 means the precision is indeterminable,
      which means compiler might using indeterminable precision during
      optimization/code gen, clang will set this value to -1 when fast
      math is enabled.

Note: Default definition float_t and double_t in current glibc:
      |  __GLIBC_FLT_EVAL_METHOD | float_t     | double_t
      |               0 or 16    | float       | double
      |               1          | double      | doulbe
      |               2          | long double | long double
      More complete list see math/math.h

Note: RISC-V has defined ISA extension to support 128-bits IEEE 754
      floating point operations, but only rare RISC-V core will implement that.

Related link:

[1] LLVM issue (__FLT_EVAL_METHOD__ is set to -1 with Ofast. #60781):
    https://github.com/llvm/llvm-project/issues/60781
[2] Last version of this patch: https://sourceware.org/pipermail/libc-alpha/2023-February/145622.html

Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
Reviewed-by: Wilco Dijkstra  <Wilco.Dijkstra@arm.com>
Link: https://inbox.sourceware.org/libc-alpha/20230314151948.12892-1-kito.cheng@sifive.com
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 bits/flt-eval-method.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/bits/flt-eval-method.h b/bits/flt-eval-method.h
index 75f57b9a0e..f9262d7d0b 100644
--- a/bits/flt-eval-method.h
+++ b/bits/flt-eval-method.h
@@ -26,14 +26,12 @@
    -1.  */
 
 /* In the default version of this header, follow __FLT_EVAL_METHOD__.
-   -1 is mapped to 2 (considering evaluation as long double to be a
-   conservatively safe assumption), and if __FLT_EVAL_METHOD__ is not
-   defined then assume there is no excess precision and use the value
-   0.  */
+   If __FLT_EVAL_METHOD__ is not defined or set to -1, assume there is no
+   excess precision and use the value 0 (this is correct for most targets). */
 
 #ifdef __FLT_EVAL_METHOD__
 # if __FLT_EVAL_METHOD__ == -1
-#  define __GLIBC_FLT_EVAL_METHOD	2
+#  define __GLIBC_FLT_EVAL_METHOD	0
 # else
 #  define __GLIBC_FLT_EVAL_METHOD	__FLT_EVAL_METHOD__
 # endif
-- 
2.40.0

Re: [COMMITTED 1/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[Joseph Myers]

How was this tested?  I've seeing test failures (for all 
build-many-glibcs.py configurations) that look like they were caused by 
this patch.

FAIL: elf/tst-glibcelf
FAIL: elf/tst-relro-ldso
FAIL: elf/tst-relro-libc

/scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:4013: error: macro STO_RISCV_VARIANT_CC redefined
/scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:3941: note: location of previous definition
/scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:4023: error: macro DT_RISCV_VARIANT_CC redefined
/scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:3937: note: location of previous definition
Traceback (most recent call last):
  File "/scratch/jmyers/glibc-bot/src/glibc/elf/tst-glibcelf.py", line 23, in <module>
    import glibcelf
  File "/scratch/jmyers/glibc-bot/src/glibc/scripts/glibcelf.py", line 226, in <module>
    _elf_h = _parse_elf_h()
  File "/scratch/jmyers/glibc-bot/src/glibc/scripts/glibcelf.py", line 223, in _parse_elf_h
    raise IOError('parse error in elf.h')
OSError: parse error in elf.h

https://sourceware.org/pipermail/libc-testresults/2023q2/011183.html

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [COMMITTED 1/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[Palmer Dabbelt]

On Fri, 28 Apr 2023 10:45:04 PDT (-0700), joseph@codesourcery.com wrote:
> How was this tested?  I've seeing test failures (for all
> build-many-glibcs.py configurations) that look like they were caused by
> this patch.
>
> FAIL: elf/tst-glibcelf
> FAIL: elf/tst-relro-ldso
> FAIL: elf/tst-relro-libc
>
> /scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:4013: error: macro STO_RISCV_VARIANT_CC redefined
> /scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:3941: note: location of previous definition
> /scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:4023: error: macro DT_RISCV_VARIANT_CC redefined
> /scratch/jmyers/glibc-bot/src/glibc/scripts/../elf/elf.h:3937: note: location of previous definition
> Traceback (most recent call last):
>   File "/scratch/jmyers/glibc-bot/src/glibc/elf/tst-glibcelf.py", line 23, in <module>
>     import glibcelf
>   File "/scratch/jmyers/glibc-bot/src/glibc/scripts/glibcelf.py", line 226, in <module>
>     _elf_h = _parse_elf_h()
>   File "/scratch/jmyers/glibc-bot/src/glibc/scripts/glibcelf.py", line 223, in _parse_elf_h
>     raise IOError('parse error in elf.h')
> OSError: parse error in elf.h
>
> https://sourceware.org/pipermail/libc-testresults/2023q2/011183.html

Sorry about that, I'd just committed it because Kito had asked me to a 
bunch of times (he's trying to sort out commit access).

I can just go revert it if you want?  Either way I'll go figure out what 
I screwed up...

Re: Fwd: [COMMITTED 1/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[Manjul Mohan]

[-- Attachment #1: Type: text/plain, Size: 5930 bytes --]


>
>
> From: Hsiangkai Wang <kai.wang@sifive.com>
>
> In some cases, we do not want to go through the resolver for function
> calls. For example, functions with vector arguments will use vector
> registers to pass arguments. In the resolver, we do not save/restore the
> vector argument registers for lazy binding efficiency. To avoid ruining
> the vector arguments, functions with vector arguments will not go
> through the resolver.
>
> To achieve the goal, we will annotate the function symbols with
> STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic
> section. In the first pass on PLT relocations, we do not set up to call
> _dl_runtime_resolve. Instead, we resolve the functions directly.
>
> Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> Link: 
> https://inbox.sourceware.org/libc-alpha/20230314162512.35802-1-kito.cheng@sifive.com
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> ---
> elf/elf.h | 7 +++++++
> manual/platform.texi | 6 ++++++
> sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++
> sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++
> 4 files changed, 60 insertions(+)
> create mode 100644 sysdeps/riscv/dl-dtprocnum.h
>
> diff --git a/elf/elf.h b/elf/elf.h
> index 94ca23c1bb..4f65b5a32d 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -3933,6 +3933,13 @@ enum
>
> #define R_TILEGX_NUM 130
>
> +/* RISC-V specific values for the Dyn d_tag field. */
> +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1)
> +#define DT_RISCV_NUM 2
> +
> +/* RISC-V specific values for the st_other field. */
> +#define STO_RISCV_VARIANT_CC 0x80


This is causing  testcase failures on ppc64le platform.

/../elf/elf.h:4013: error: macro STO_RISCV_VARIANT_CC redefined
//../elf/elf.h:3941: note: location of previous definition
../elf/elf.h:4023: error: macro DT_RISCV_VARIANT_CC redefined
../elf/elf.h:3937: note: location of previous definition

Testcase failures:
elf/tst-glibcelf
elf/tst-relro-ldso
elf/tst-relro-libc

> +
> /* RISC-V ELF Flags */
> #define EF_RISCV_RVC 0x0001
> #define EF_RISCV_FLOAT_ABI 0x0006
> diff --git a/manual/platform.texi b/manual/platform.texi
> index c6ed73cb97..714c07813f 100644
> --- a/manual/platform.texi
> +++ b/manual/platform.texi
> @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to 
> medium.
> @node RISC-V
> @appendixsec RISC-V-specific Facilities
>
> +Functions that are lazily bound must be compatible with the standard 
> calling
> +convention. When a function is annotated with STO_RISCV_VARIANT_CC, 
> it means
> +this function is not compatible with the standard calling convention. The
> +dynamic linker will directly resolve it instead of using the lazy binding
> +mechanism.
> +
> Cache management facilities specific to RISC-V systems that implement 
> the Linux
> ABI are declared in @file{sys/cachectl.h}.
>
> diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h
> new file mode 100644
> index 0000000000..281c5aadeb
> --- /dev/null
> +++ b/sysdeps/riscv/dl-dtprocnum.h
> @@ -0,0 +1,21 @@
> +/* Configuration of lookup functions. RISC-V version.
> + Copyright (C) 2023 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +/* Number of extra dynamic section entries for this architecture. By
> + default there are none. */
> +#define DT_THISPROCNUM DT_RISCV_NUM
> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> index c0c9bd93ad..0e6c0bb155 100644
> --- a/sysdeps/riscv/dl-machine.h
> +++ b/sysdeps/riscv/dl-machine.h
> @@ -53,6 +53,9 @@
> || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \
> | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
>
> +//* Translate a processor specific dynamic tag to the index in l_info 
> array. */
> +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM)
> +
> /* Return nonzero iff ELF header is compatible with the running host. */
> static inline int __attribute_used__
> elf_machine_matches_host (const ElfW(Ehdr) *ehdr)
> @@ -281,6 +284,29 @@ elf_machine_lazy_rel (struct link_map *map, 
> struct r_scope_elem *scope[],
> /* Check for unexpected PLT reloc type. */
> if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT))
> {
> + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL))
> + {
> + /* Check the symbol table for variant CC symbols. */
> + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info);
> + const ElfW(Sym) *symtab =
> + (const void *)D_PTR (map, l_info[DT_SYMTAB]);
> + const ElfW(Sym) *sym = &symtab[symndx];
> + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC))
> + {
> + /* Avoid lazy resolution of variant CC symbols. */
> + const struct r_found_version *version = NULL;
> + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
> + {
> + const ElfW(Half) *vernum =
> + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]);
> + version = &map->l_versions[vernum[symndx] & 0x7fff];
> + }
> + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr,
> + skip_ifunc);
> + return;
> + }
> + }
> +
> if (__glibc_unlikely (map->l_mach.plt == 0))
> {
> if (l_addr)
> -- 
> 2.40.0
>