[PATCH] elfedit: add support for editing e_flags

[PATCH] elfedit: add support for editing e_flags

[WANG Xuerui]

From: WANG Xuerui <git@xen0n.name>

Add a pair of elfedit options (--{input,output}-flags) for manipulating
files' e_flags field. This is useful in certain cases, e.g. if you know
what you are doing and have to make some object files look as if
compiled for a different architecture or ABI variant.
---

I have to pick up the work because I was trying to make AMDGPU DCN work
on LoongArch, but that piece of code requires hard-float support while
the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
link-time failures intermixing the two. Since the hard-float code in
question doesn't pass around FP arguments across the boundary between
object files, but rather only internally, it is safe to just edit the
e_flags of the hard-float objects to make their ABI look like LP64S.
Which led to me discovering the previous similar work [1] has stalled...
And that's why a bunch of LoongArch-related folks are on the CC list.

[1]: https://sourceware.org/pipermail/binutils/2022-April/120413.html

 binutils/doc/binutils.texi                  | 13 +++++-
 binutils/elfedit.c                          | 52 +++++++++++++++++++--
 binutils/testsuite/binutils-all/elfedit-7.d | 10 ++++
 binutils/testsuite/binutils-all/elfedit-8.d | 10 ++++
 binutils/testsuite/binutils-all/elfedit.exp |  2 +
 5 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/elfedit-7.d
 create mode 100644 binutils/testsuite/binutils-all/elfedit-8.d

diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index b1982a95704..a261af48254 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -5297,10 +5297,12 @@ elfedit [@option{--input-mach=}@var{machine}]
         [@option{--input-type=}@var{type}]
         [@option{--input-osabi=}@var{osabi}]
         [@option{--input-abiversion=}@var{version}]
+        [@option{--input-flags=}@var{flags}]
         @option{--output-mach=}@var{machine}
         @option{--output-type=}@var{type}
         @option{--output-osabi=}@var{osabi}
         @option{--output-abiversion=}@var{version}
+        @option{--output-flags=}@var{flags}
         @option{--enable-x86-feature=}@var{feature}
         @option{--disable-x86-feature=}@var{feature}
         [@option{-v}|@option{--version}]
@@ -5325,7 +5327,7 @@ should be updated.
 The long and short forms of options, shown here as alternatives, are
 equivalent. At least one of the @option{--output-mach},
 @option{--output-type}, @option{--output-osabi},
-@option{--output-abiversion},
+@option{--output-abiversion}, @option{--output-flags},
 @option{--enable-x86-feature} and @option{--disable-x86-feature}
 options must be given.
 
@@ -5376,6 +5378,15 @@ isn't specified, it will match any ELF ABIVERSIONs.
 Change the ELF ABIVERSION in the ELF header to @var{version}.
 @var{version} must be between 0 and 255.
 
+@item --input-flags=@var{flags}
+Set the matching input ELF file @var{e_flags} value to @var{flags}.
+@var{flags} must be an unsigned 32-bit integer.  If @option{--input-flags}
+isn't specified, it will match any e_flags value.
+
+@item --output-flags=@var{flags}
+Change the @var{e_flags} field in the ELF header to @var{flags}.
+@var{version} must be an unsigned 32-bit integer.
+
 @item --enable-x86-feature=@var{feature}
 Set the @var{feature} bit in program property in @var{exec} or @var{dyn}
 ELF files with machine types of @var{i386} or @var{x86-64}.  The
diff --git a/binutils/elfedit.c b/binutils/elfedit.c
index 117e67639a0..512e075f16a 100644
--- a/binutils/elfedit.c
+++ b/binutils/elfedit.c
@@ -68,6 +68,10 @@ enum elfclass
   };
 static enum elfclass input_elf_class = ELF_CLASS_UNKNOWN;
 static enum elfclass output_elf_class = ELF_CLASS_BOTH;
+static int check_elf_flags = 0;
+static unsigned long input_elf_flags = 0;
+static int write_elf_flags = 0;
+static unsigned long output_elf_flags = 0;
 
 #ifdef HAVE_MMAP
 #include <sys/mman.h>
@@ -394,7 +398,16 @@ update_elf_header (const char *file_name, FILE *file)
       return 0;
     }
 
-  /* Update e_machine, e_type and EI_OSABI.  */
+  /* Skip if e_flags doesn't match. */
+  if (check_elf_flags && elf_header.e_flags != input_elf_flags)
+    {
+      error
+	(_("%s: Unmatched e_flags: 0x%lx is not 0x%lx\n"),
+	 file_name, elf_header.e_flags, input_elf_flags);
+      return 0;
+    }
+
+  /* Update e_machine, e_type, OSABI, ABIVERSION and e_flags.  */
   switch (class)
     {
     default:
@@ -410,6 +423,8 @@ update_elf_header (const char *file_name, FILE *file)
 	ehdr32.e_ident[EI_OSABI] = output_elf_osabi;
       if (output_elf_abiversion != -1)
 	ehdr32.e_ident[EI_ABIVERSION] = output_elf_abiversion;
+      if (write_elf_flags)
+	BYTE_PUT (ehdr32.e_flags, output_elf_flags);
       status = fwrite (&ehdr32, sizeof (ehdr32), 1, file) == 1;
       break;
     case ELFCLASS64:
@@ -421,6 +436,8 @@ update_elf_header (const char *file_name, FILE *file)
 	ehdr64.e_ident[EI_OSABI] = output_elf_osabi;
       if (output_elf_abiversion != -1)
 	ehdr64.e_ident[EI_ABIVERSION] = output_elf_abiversion;
+      if (write_elf_flags)
+	BYTE_PUT (ehdr64.e_flags, output_elf_flags);
       status = fwrite (&ehdr64, sizeof (ehdr64), 1, file) == 1;
       break;
     }
@@ -904,6 +921,8 @@ enum command_line_switch
     OPTION_OUTPUT_OSABI,
     OPTION_INPUT_ABIVERSION,
     OPTION_OUTPUT_ABIVERSION,
+    OPTION_INPUT_FLAGS,
+    OPTION_OUTPUT_FLAGS,
 #ifdef HAVE_MMAP
     OPTION_ENABLE_X86_FEATURE,
     OPTION_DISABLE_X86_FEATURE,
@@ -920,6 +939,8 @@ static struct option options[] =
   {"output-osabi",	required_argument, 0, OPTION_OUTPUT_OSABI},
   {"input-abiversion",	required_argument, 0, OPTION_INPUT_ABIVERSION},
   {"output-abiversion",	required_argument, 0, OPTION_OUTPUT_ABIVERSION},
+  {"input-flags",	required_argument, 0, OPTION_INPUT_FLAGS},
+  {"output-flags",	required_argument, 0, OPTION_OUTPUT_FLAGS},
 #ifdef HAVE_MMAP
   {"enable-x86-feature",
 			required_argument, 0, OPTION_ENABLE_X86_FEATURE},
@@ -958,7 +979,11 @@ usage (FILE *stream, int exit_status)
   --output-osabi [%s]\n\
                               Set output OSABI\n\
   --input-abiversion [0-255]  Set input ABIVERSION\n\
-  --output-abiversion [0-255] Set output ABIVERSION\n"),
+  --output-abiversion [0-255] Set output ABIVERSION\n\
+  --input-flags [32-bit unsigned integer]\n\
+                              Set input e_flags\n\
+  --output-flags [32-bit unsigned integer]\n\
+                              Set output e_flags\n"),
 	   osabi, osabi);
 #ifdef HAVE_MMAP
   fprintf (stream, _("\
@@ -1062,6 +1087,26 @@ main (int argc, char ** argv)
 	    }
 	  break;
 
+	case OPTION_INPUT_FLAGS:
+	  check_elf_flags = 1;
+	  input_elf_flags = strtoul (optarg, &end, 0);
+	  if (*end != '\0' || input_elf_flags > 0xffffffff)
+	    {
+	      error (_("Invalid e_flags: %s\n"), optarg);
+	      return 1;
+	    }
+	  break;
+
+	case OPTION_OUTPUT_FLAGS:
+	  write_elf_flags = 1;
+	  output_elf_flags = strtoul (optarg, &end, 0);
+	  if (*end != '\0' || output_elf_flags > 0xffffffff)
+	    {
+	      error (_("Invalid e_flags: %s\n"), optarg);
+	      return 1;
+	    }
+	  break;
+
 #ifdef HAVE_MMAP
 	case OPTION_ENABLE_X86_FEATURE:
 	  if (elf_x86_feature (optarg, 1) < 0)
@@ -1094,7 +1139,8 @@ main (int argc, char ** argv)
 #endif
 	  && output_elf_type == -1
 	  && output_elf_osabi == -1
-	  && output_elf_abiversion == -1))
+	  && output_elf_abiversion == -1
+	  && ! write_elf_flags))
     usage (stderr, 1);
 
   status = 0;
diff --git a/binutils/testsuite/binutils-all/elfedit-7.d b/binutils/testsuite/binutils-all/elfedit-7.d
new file mode 100644
index 00000000000..8e5a8b7d019
--- /dev/null
+++ b/binutils/testsuite/binutils-all/elfedit-7.d
@@ -0,0 +1,10 @@
+#PROG: elfedit
+#elfedit: --output-flags 0xfedcba98
+#source: empty.s
+#readelf: -h
+#name: Update ELF header 7 (hexadecimal e_flags value)
+#target: *-*-linux* *-*-gnu*
+
+#...
+  Flags:[ \t]+0xfedcba98.*
+#...
diff --git a/binutils/testsuite/binutils-all/elfedit-8.d b/binutils/testsuite/binutils-all/elfedit-8.d
new file mode 100644
index 00000000000..9538f4486b8
--- /dev/null
+++ b/binutils/testsuite/binutils-all/elfedit-8.d
@@ -0,0 +1,10 @@
+#PROG: elfedit
+#elfedit: --output-flags 12345678
+#source: empty.s
+#readelf: -h
+#name: Update ELF header 8 (decimal e_flags value)
+#target: *-*-linux* *-*-gnu*
+
+#...
+  Flags:[ \t]+0xbc614e.*
+#...
diff --git a/binutils/testsuite/binutils-all/elfedit.exp b/binutils/testsuite/binutils-all/elfedit.exp
index 33f4bb05529..47e94e5a775 100644
--- a/binutils/testsuite/binutils-all/elfedit.exp
+++ b/binutils/testsuite/binutils-all/elfedit.exp
@@ -26,3 +26,5 @@ run_dump_test "elfedit-3"
 run_dump_test "elfedit-4"
 run_dump_test "elfedit-5"
 run_dump_test "elfedit-6"
+run_dump_test "elfedit-7"
+run_dump_test "elfedit-8"
-- 
2.39.1

Re: [PATCH] elfedit: add support for editing e_flags

[Xi Ruoyao]

On Thu, 2023-03-02 at 16:01 +0800, WANG Xuerui wrote:

> I have to pick up the work because I was trying to make AMDGPU DCN work
> on LoongArch, but that piece of code requires hard-float support while
> the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
> link-time failures intermixing the two.

IMO we should keep soft-float ABI while enabling the FPU (i. e.
-mabi=lp64s -mfpu=64).  But ouch, this does not work properly with
current GCC...  -mfpu=64 silently enables using FPR to pass arg and
return value, w/o even a warning.  Not sure about Clang.

<rant>I remember I've been saying "floating ABI and floating instruction
set should be de-coupled" multiple times: "I" in "ABI" stands for
"interface" anyway and all the interfaces are just designed for hiding
implementation details.  But it seems nobody took it seriously.</rant>

> Since the hard-float code in
> question doesn't pass around FP arguments across the boundary between
> object files, but rather only internally, it is safe to just edit the
> e_flags of the hard-float objects to make their ABI look like LP64S.
> Which led to me discovering the previous similar work [1] has stalled...
> And that's why a bunch of LoongArch-related folks are on the CC list.
> 
> [1]: https://sourceware.org/pipermail/binutils/2022-April/120413.html
> 
>  binutils/doc/binutils.texi                  | 13 +++++-
>  binutils/elfedit.c                          | 52 +++++++++++++++++++-
> -
>  binutils/testsuite/binutils-all/elfedit-7.d | 10 ++++
>  binutils/testsuite/binutils-all/elfedit-8.d | 10 ++++
>  binutils/testsuite/binutils-all/elfedit.exp |  2 +
>  5 files changed, 83 insertions(+), 4 deletions(-)
>  create mode 100644 binutils/testsuite/binutils-all/elfedit-7.d
>  create mode 100644 binutils/testsuite/binutils-all/elfedit-8.d
> 
> diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> index b1982a95704..a261af48254 100644
> --- a/binutils/doc/binutils.texi
> +++ b/binutils/doc/binutils.texi
> @@ -5297,10 +5297,12 @@ elfedit [@option{--input-mach=}@var{machine}]
>          [@option{--input-type=}@var{type}]
>          [@option{--input-osabi=}@var{osabi}]
>          [@option{--input-abiversion=}@var{version}]
> +        [@option{--input-flags=}@var{flags}]
>          @option{--output-mach=}@var{machine}
>          @option{--output-type=}@var{type}
>          @option{--output-osabi=}@var{osabi}
>          @option{--output-abiversion=}@var{version}
> +        @option{--output-flags=}@var{flags}
>          @option{--enable-x86-feature=}@var{feature}
>          @option{--disable-x86-feature=}@var{feature}
>          [@option{-v}|@option{--version}]
> @@ -5325,7 +5327,7 @@ should be updated.
>  The long and short forms of options, shown here as alternatives, are
>  equivalent. At least one of the @option{--output-mach},
>  @option{--output-type}, @option{--output-osabi},
> -@option{--output-abiversion},
> +@option{--output-abiversion}, @option{--output-flags},
>  @option{--enable-x86-feature} and @option{--disable-x86-feature}
>  options must be given.
>  
> @@ -5376,6 +5378,15 @@ isn't specified, it will match any ELF
> ABIVERSIONs.
>  Change the ELF ABIVERSION in the ELF header to @var{version}.
>  @var{version} must be between 0 and 255.
>  
> +@item --input-flags=@var{flags}
> +Set the matching input ELF file @var{e_flags} value to @var{flags}.
> +@var{flags} must be an unsigned 32-bit integer.  If @option{--input-
> flags}
> +isn't specified, it will match any e_flags value.
> +
> +@item --output-flags=@var{flags}
> +Change the @var{e_flags} field in the ELF header to @var{flags}.
> +@var{version} must be an unsigned 32-bit integer.
> +
>  @item --enable-x86-feature=@var{feature}
>  Set the @var{feature} bit in program property in @var{exec} or
> @var{dyn}
>  ELF files with machine types of @var{i386} or @var{x86-64}.  The
> diff --git a/binutils/elfedit.c b/binutils/elfedit.c
> index 117e67639a0..512e075f16a 100644
> --- a/binutils/elfedit.c
> +++ b/binutils/elfedit.c
> @@ -68,6 +68,10 @@ enum elfclass
>    };
>  static enum elfclass input_elf_class = ELF_CLASS_UNKNOWN;
>  static enum elfclass output_elf_class = ELF_CLASS_BOTH;
> +static int check_elf_flags = 0;
> +static unsigned long input_elf_flags = 0;
> +static int write_elf_flags = 0;
> +static unsigned long output_elf_flags = 0;
>  
>  #ifdef HAVE_MMAP
>  #include <sys/mman.h>
> @@ -394,7 +398,16 @@ update_elf_header (const char *file_name, FILE
> *file)
>        return 0;
>      }
>  
> -  /* Update e_machine, e_type and EI_OSABI.  */
> +  /* Skip if e_flags doesn't match. */
> +  if (check_elf_flags && elf_header.e_flags != input_elf_flags)
> +    {
> +      error
> +       (_("%s: Unmatched e_flags: 0x%lx is not 0x%lx\n"),
> +        file_name, elf_header.e_flags, input_elf_flags);
> +      return 0;
> +    }
> +
> +  /* Update e_machine, e_type, OSABI, ABIVERSION and e_flags.  */
>    switch (class)
>      {
>      default:
> @@ -410,6 +423,8 @@ update_elf_header (const char *file_name, FILE
> *file)
>         ehdr32.e_ident[EI_OSABI] = output_elf_osabi;
>        if (output_elf_abiversion != -1)
>         ehdr32.e_ident[EI_ABIVERSION] = output_elf_abiversion;
> +      if (write_elf_flags)
> +       BYTE_PUT (ehdr32.e_flags, output_elf_flags);
>        status = fwrite (&ehdr32, sizeof (ehdr32), 1, file) == 1;
>        break;
>      case ELFCLASS64:
> @@ -421,6 +436,8 @@ update_elf_header (const char *file_name, FILE
> *file)
>         ehdr64.e_ident[EI_OSABI] = output_elf_osabi;
>        if (output_elf_abiversion != -1)
>         ehdr64.e_ident[EI_ABIVERSION] = output_elf_abiversion;
> +      if (write_elf_flags)
> +       BYTE_PUT (ehdr64.e_flags, output_elf_flags);
>        status = fwrite (&ehdr64, sizeof (ehdr64), 1, file) == 1;
>        break;
>      }
> @@ -904,6 +921,8 @@ enum command_line_switch
>      OPTION_OUTPUT_OSABI,
>      OPTION_INPUT_ABIVERSION,
>      OPTION_OUTPUT_ABIVERSION,
> +    OPTION_INPUT_FLAGS,
> +    OPTION_OUTPUT_FLAGS,
>  #ifdef HAVE_MMAP
>      OPTION_ENABLE_X86_FEATURE,
>      OPTION_DISABLE_X86_FEATURE,
> @@ -920,6 +939,8 @@ static struct option options[] =
>    {"output-osabi",     required_argument, 0, OPTION_OUTPUT_OSABI},
>    {"input-abiversion", required_argument, 0,
> OPTION_INPUT_ABIVERSION},
>    {"output-abiversion",        required_argument, 0,
> OPTION_OUTPUT_ABIVERSION},
> +  {"input-flags",      required_argument, 0, OPTION_INPUT_FLAGS},
> +  {"output-flags",     required_argument, 0, OPTION_OUTPUT_FLAGS},
>  #ifdef HAVE_MMAP
>    {"enable-x86-feature",
>                         required_argument, 0,
> OPTION_ENABLE_X86_FEATURE},
> @@ -958,7 +979,11 @@ usage (FILE *stream, int exit_status)
>    --output-osabi [%s]\n\
>                                Set output OSABI\n\
>    --input-abiversion [0-255]  Set input ABIVERSION\n\
> -  --output-abiversion [0-255] Set output ABIVERSION\n"),
> +  --output-abiversion [0-255] Set output ABIVERSION\n\
> +  --input-flags [32-bit unsigned integer]\n\
> +                              Set input e_flags\n\
> +  --output-flags [32-bit unsigned integer]\n\
> +                              Set output e_flags\n"),
>            osabi, osabi);
>  #ifdef HAVE_MMAP
>    fprintf (stream, _("\
> @@ -1062,6 +1087,26 @@ main (int argc, char ** argv)
>             }
>           break;
>  
> +       case OPTION_INPUT_FLAGS:
> +         check_elf_flags = 1;
> +         input_elf_flags = strtoul (optarg, &end, 0);
> +         if (*end != '\0' || input_elf_flags > 0xffffffff)
> +           {
> +             error (_("Invalid e_flags: %s\n"), optarg);
> +             return 1;
> +           }
> +         break;
> +
> +       case OPTION_OUTPUT_FLAGS:
> +         write_elf_flags = 1;
> +         output_elf_flags = strtoul (optarg, &end, 0);
> +         if (*end != '\0' || output_elf_flags > 0xffffffff)
> +           {
> +             error (_("Invalid e_flags: %s\n"), optarg);
> +             return 1;
> +           }
> +         break;
> +
>  #ifdef HAVE_MMAP
>         case OPTION_ENABLE_X86_FEATURE:
>           if (elf_x86_feature (optarg, 1) < 0)
> @@ -1094,7 +1139,8 @@ main (int argc, char ** argv)
>  #endif
>           && output_elf_type == -1
>           && output_elf_osabi == -1
> -         && output_elf_abiversion == -1))
> +         && output_elf_abiversion == -1
> +         && ! write_elf_flags))
>      usage (stderr, 1);
>  
>    status = 0;
> diff --git a/binutils/testsuite/binutils-all/elfedit-7.d
> b/binutils/testsuite/binutils-all/elfedit-7.d
> new file mode 100644
> index 00000000000..8e5a8b7d019
> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/elfedit-7.d
> @@ -0,0 +1,10 @@
> +#PROG: elfedit
> +#elfedit: --output-flags 0xfedcba98
> +#source: empty.s
> +#readelf: -h
> +#name: Update ELF header 7 (hexadecimal e_flags value)
> +#target: *-*-linux* *-*-gnu*
> +
> +#...
> +  Flags:[ \t]+0xfedcba98.*
> +#...
> diff --git a/binutils/testsuite/binutils-all/elfedit-8.d
> b/binutils/testsuite/binutils-all/elfedit-8.d
> new file mode 100644
> index 00000000000..9538f4486b8
> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/elfedit-8.d
> @@ -0,0 +1,10 @@
> +#PROG: elfedit
> +#elfedit: --output-flags 12345678
> +#source: empty.s
> +#readelf: -h
> +#name: Update ELF header 8 (decimal e_flags value)
> +#target: *-*-linux* *-*-gnu*
> +
> +#...
> +  Flags:[ \t]+0xbc614e.*
> +#...
> diff --git a/binutils/testsuite/binutils-all/elfedit.exp
> b/binutils/testsuite/binutils-all/elfedit.exp
> index 33f4bb05529..47e94e5a775 100644
> --- a/binutils/testsuite/binutils-all/elfedit.exp
> +++ b/binutils/testsuite/binutils-all/elfedit.exp
> @@ -26,3 +26,5 @@ run_dump_test "elfedit-3"
>  run_dump_test "elfedit-4"
>  run_dump_test "elfedit-5"
>  run_dump_test "elfedit-6"
> +run_dump_test "elfedit-7"
> +run_dump_test "elfedit-8"

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] elfedit: add support for editing e_flags

[WANG Xuerui]

On 2023/3/2 16:20, Xi Ruoyao wrote:
> On Thu, 2023-03-02 at 16:01 +0800, WANG Xuerui wrote:
>
>> I have to pick up the work because I was trying to make AMDGPU DCN work
>> on LoongArch, but that piece of code requires hard-float support while
>> the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
>> link-time failures intermixing the two.
> IMO we should keep soft-float ABI while enabling the FPU (i. e.
> -mabi=lp64s -mfpu=64).  But ouch, this does not work properly with
> current GCC...  -mfpu=64 silently enables using FPR to pass arg and
> return value, w/o even a warning.  Not sure about Clang.
>
> <rant>I remember I've been saying "floating ABI and floating instruction
> set should be de-coupled" multiple times: "I" in "ABI" stands for
> "interface" anyway and all the interfaces are just designed for hiding
> implementation details.  But it seems nobody took it seriously.</rant>

Yeah. I remember the Toolchain Conventions has similar words so I've 
immediately tried `-mdouble-float` without `-mabi=lp64d`, after seeing 
the rejected interlinks, only to see the ABI implicitly promoted to 
LP64D nevertheless...

Re: [PATCH] elfedit: add support for editing e_flags

[Xi Ruoyao]

On Thu, 2023-03-02 at 16:53 +0800, WANG Xuerui wrote:
> On 2023/3/2 16:20, Xi Ruoyao wrote:
> > On Thu, 2023-03-02 at 16:01 +0800, WANG Xuerui wrote:
> > 
> > > I have to pick up the work because I was trying to make AMDGPU DCN work
> > > on LoongArch, but that piece of code requires hard-float support while
> > > the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
> > > link-time failures intermixing the two.
> > IMO we should keep soft-float ABI while enabling the FPU (i. e.
> > -mabi=lp64s -mfpu=64).  But ouch, this does not work properly with
> > current GCC...  -mfpu=64 silently enables using FPR to pass arg and
> > return value, w/o even a warning.  Not sure about Clang.
> > 
> > <rant>I remember I've been saying "floating ABI and floating instruction
> > set should be de-coupled" multiple times: "I" in "ABI" stands for
> > "interface" anyway and all the interfaces are just designed for hiding
> > implementation details.  But it seems nobody took it seriously.</rant>
> 
> Yeah. I remember the Toolchain Conventions has similar words so I've 
> immediately tried `-mdouble-float` without `-mabi=lp64d`, after seeing
> the rejected interlinks, only to see the ABI implicitly promoted to 
> LP64D nevertheless...

If I read the doc correctly, -mdouble-float implies both -mfpu=64 and -
mabi=lpxxd, while -mfpu=64 alone should not affect the calling
convention unless it's absolutely necessary (for e.g. -mfpu=32 -
mabi=lp64d can downgrade the ABI to lp64f).

I consider "-mfpu=64 silently changes -mabi=lp64s to -mabi=lp64d" a GCC
bug (behavior difference from the toolchain convention), now making a
GCC patch...
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] elfedit: add support for editing e_flags

[Xi Ruoyao]

On Thu, 2023-03-02 at 17:04 +0800, Xi Ruoyao wrote:
> On Thu, 2023-03-02 at 16:53 +0800, WANG Xuerui wrote:
> > On 2023/3/2 16:20, Xi Ruoyao wrote:
> > > On Thu, 2023-03-02 at 16:01 +0800, WANG Xuerui wrote:
> > > 
> > > > I have to pick up the work because I was trying to make AMDGPU DCN work
> > > > on LoongArch, but that piece of code requires hard-float support while
> > > > the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
> > > > link-time failures intermixing the two.
> > > IMO we should keep soft-float ABI while enabling the FPU (i. e.
> > > -mabi=lp64s -mfpu=64).  But ouch, this does not work properly with
> > > current GCC...  -mfpu=64 silently enables using FPR to pass arg and
> > > return value, w/o even a warning.  Not sure about Clang.
> > > 
> > > <rant>I remember I've been saying "floating ABI and floating instruction
> > > set should be de-coupled" multiple times: "I" in "ABI" stands for
> > > "interface" anyway and all the interfaces are just designed for hiding
> > > implementation details.  But it seems nobody took it seriously.</rant>
> > 
> > Yeah. I remember the Toolchain Conventions has similar words so I've
> > immediately tried `-mdouble-float` without `-mabi=lp64d`, after seeing
> > the rejected interlinks, only to see the ABI implicitly promoted to 
> > LP64D nevertheless...

Try "-mabi=lp64s -mfpu=64".  It should create object files with SOFT-
FLOAT in flags.

The current GCC behavior is still problematic as it actually uses LP64D
ABI, though marking the file as LP64S.  Example:

$ cat t.c
double t(double x)
{
	return 1.0 / x;
}
$ cc t.c -O2 -mabi=lp64s -mfpu=64 -c
$ readelf -a t.o | grep FLOAT    
  Flags:                             0x41, SOFT-FLOAT, OBJ-v1

But then bad thing happens:

$ objdump -d t.o

t.o:     file format elf64-loongarch


Disassembly of section .text:

0000000000000000 <t>:
   0:	01145800 	frecip.d    	$fa0, $fa0     <= you can't expect the arg in the god-damn $fa0!
   4:	4c000020 	jirl        	$zero, $ra, 0  <= you can't just return the answer in $fa0!

This is completely broken.  However if your elfedit hack will "work",
this will "work" too anyway.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] elfedit: add support for editing e_flags

[WANG Xuerui]

On 2023/3/2 18:42, Xi Ruoyao wrote:
> On Thu, 2023-03-02 at 17:04 +0800, Xi Ruoyao wrote:
>> On Thu, 2023-03-02 at 16:53 +0800, WANG Xuerui wrote:
>>> On 2023/3/2 16:20, Xi Ruoyao wrote:
>>>> On Thu, 2023-03-02 at 16:01 +0800, WANG Xuerui wrote:
>>>>
>>>>> I have to pick up the work because I was trying to make AMDGPU DCN work
>>>>> on LoongArch, but that piece of code requires hard-float support while
>>>>> the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
>>>>> link-time failures intermixing the two.
>>>> IMO we should keep soft-float ABI while enabling the FPU (i. e.
>>>> -mabi=lp64s -mfpu=64).  But ouch, this does not work properly with
>>>> current GCC...  -mfpu=64 silently enables using FPR to pass arg and
>>>> return value, w/o even a warning.  Not sure about Clang.
>>>>
>>>> <rant>I remember I've been saying "floating ABI and floating instruction
>>>> set should be de-coupled" multiple times: "I" in "ABI" stands for
>>>> "interface" anyway and all the interfaces are just designed for hiding
>>>> implementation details.  But it seems nobody took it seriously.</rant>
>>> Yeah. I remember the Toolchain Conventions has similar words so I've
>>> immediately tried `-mdouble-float` without `-mabi=lp64d`, after seeing
>>> the rejected interlinks, only to see the ABI implicitly promoted to
>>> LP64D nevertheless...
> Try "-mabi=lp64s -mfpu=64".  It should create object files with SOFT-
> FLOAT in flags.

Hmm yeah you're right. The LoongArch kernel CFLAGS includes -msoft-float 
though, so I can confirm things work as intended after removing that flag:

# drivers/gpu/drm/amd/display/dc/dml/Makefile
dml_ccflags := -mabi=lp64s -mfpu=64
dml_rcflags := -msoft-float # "rcflags" means "removing from cflags" 
apparently

Indeed the object files are correctly LP64S with FP instructions. Won't 
have to ask every LoongArch distro to upgrade toolchains any more...

>
> The current GCC behavior is still problematic as it actually uses LP64D
> ABI, though marking the file as LP64S.  Example:
>
> $ cat t.c
> double t(double x)
> {
> 	return 1.0 / x;
> }
> $ cc t.c -O2 -mabi=lp64s -mfpu=64 -c
> $ readelf -a t.o | grep FLOAT
>    Flags:                             0x41, SOFT-FLOAT, OBJ-v1
>
> But then bad thing happens:
>
> $ objdump -d t.o
>
> t.o:     file format elf64-loongarch
>
>
> Disassembly of section .text:
>
> 0000000000000000 <t>:
>     0:	01145800 	frecip.d    	$fa0, $fa0     <= you can't expect the arg in the god-damn $fa0!
>     4:	4c000020 	jirl        	$zero, $ra, 0  <= you can't just return the answer in $fa0!
>
> This is completely broken.  However if your elfedit hack will "work",
> this will "work" too anyway.
>
Mixing calling conventions is currently problematic in general, true. 
But in amdgpu's case things will work just fine, because discipline is 
used throughout to ensure (a) no FP argument is passed across the 
boundary between FPU-enabled code and the rest, and (b) any such 
boundary-crossing method takes care to save/restore FPU context 
appropriately. You can argue this is the general case in kernel land, even.

But again, thanks for your help checking all of this!

Re: [PATCH] elfedit: add support for editing e_flags

[Xi Ruoyao]

On Thu, 2023-03-02 at 19:02 +0800, WANG Xuerui wrote:
> Indeed the object files are correctly LP64S with FP instructions. Won't 
> have to ask every LoongArch distro to upgrade toolchains any more...

You may need to fix and/or upgrade Clang if you want to build the DCN
driver with it.  A Clang "14.0.6" (no idea what it really is, IIRC even
Clang 15 does not support LoongArch) on gcc400.fsffrance.org (Arch Linux
installed) produces:

[xry111@gcc400 ~]$ clang t.c -c -mabi=lp64s -mfpu=64
[xry111@gcc400 ~]$ readelf -a a.out | grep FLOAT
  Flags:                             0x43, DOUBLE-FLOAT, OBJ-v1

And there is some more strange thing:

[xry111@gcc400 ~]$ clang t.c -c -mabi=lp64s -mfpu=32
[xry111@gcc400 ~]$ readelf -a a.out | grep FLOAT
  Flags:                             0x43, DOUBLE-FLOAT, OBJ-v1

Why would "lp64s" and "fpu=32" lead to double float ?!

Cc += Weining, though I'm not sure if this issue still exists in LLVM
mainline.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] elfedit: add support for editing e_flags

[Xi Ruoyao]

On Thu, 2023-03-02 at 19:02 +0800, WANG Xuerui wrote:
> > $ cc t.c -O2 -mabi=lp64s -mfpu=64 -c
> > $ readelf -a t.o | grep FLOAT
> >     Flags:                             0x41, SOFT-FLOAT, OBJ-v1
> > 
> > But then bad thing happens:
> > 
> > $ objdump -d t.o
> > 
> > t.o:     file format elf64-loongarch
> > 
> > 
> > Disassembly of section .text:
> > 
> > 0000000000000000 <t>:
> >      0: 01145800        frecip.d        $fa0, $fa0     <= you can't expect the arg in the god-damn $fa0!
> >      4: 4c000020        jirl            $zero, $ra, 0  <= you can't just return the answer in $fa0!
> > 
> > This is completely broken.  However if your elfedit hack will "work",
> > this will "work" too anyway.

FWIW: GCC patch is now
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613215.html.

> Mixing calling conventions is currently problematic in general, true. 
> But in amdgpu's case things will work just fine, because discipline is
> used throughout to ensure (a) no FP argument is passed across the 
> boundary between FPU-enabled code and the rest, and (b) any such 
> boundary-crossing method takes care to save/restore FPU context 
> appropriately. You can argue this is the general case in kernel land, even.

Yes, it seems some other architectures are using this trick for DCN.

(I'd planned to port the DCN driver several weeks later, but I guess
you've already completed it now.  And frankly I didn't expect the
misbehavior of GCC :)
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: Re: [PATCH] elfedit: add support for editing e_flags

[Weining Lu]

LLVM mainline doesn't have such an issue. The output is:
$ clang -c t.c -mabi=lp64s -mfpu=64
warning: triple-implied ABI conflicts with provided target-abi 'lp64s', using target-abi
warning: triple-implied ABI conflicts with provided target-abi 'lp64s', using target-abi
$ readelf -h t.o | grep FLOAT
  Flags:                             0x41, SOFT-FLOAT, OBJ-v1
$ clang -c t.c -mabi=lp64s -mfpu=32
warning: triple-implied ABI conflicts with provided target-abi 'lp64s', using target-abi
warning: triple-implied ABI conflicts with provided target-abi 'lp64s', using target-abi
$ readelf -h t.o | grep FLOAT
  Flags:                             0x41, SOFT-FLOAT, OBJ-v1

> -----原始邮件-----
> 发件人: "Xi Ruoyao" <xry111@xry111.site>
> 发送时间:2023-03-02 19:21:43 (星期四)
> 收件人: "WANG Xuerui" <i.swmail@xen0n.name>, binutils@sourceware.org
> 抄送: "Chenghua Xu" <xuchenghua@loongson.cn>, "Zhensong Liu" <liuzhensong@loongson.cn>, "Qinggang Meng" <mengqinggang@loongson.cn>, "WANG
>  Xuerui" <git@xen0n.name>, "Weining Lu" <luweining@loongson.cn>
> 主题: Re: [PATCH] elfedit: add support for editing e_flags
> 
> On Thu, 2023-03-02 at 19:02 +0800, WANG Xuerui wrote:
> > Indeed the object files are correctly LP64S with FP instructions. Won't 
> > have to ask every LoongArch distro to upgrade toolchains any more...
> 
> You may need to fix and/or upgrade Clang if you want to build the DCN
> driver with it.  A Clang "14.0.6" (no idea what it really is, IIRC even
> Clang 15 does not support LoongArch) on gcc400.fsffrance.org (Arch Linux
> installed) produces:
> 
> [xry111@gcc400 ~]$ clang t.c -c -mabi=lp64s -mfpu=64
> [xry111@gcc400 ~]$ readelf -a a.out | grep FLOAT
>   Flags:                             0x43, DOUBLE-FLOAT, OBJ-v1
> 
> And there is some more strange thing:
> 
> [xry111@gcc400 ~]$ clang t.c -c -mabi=lp64s -mfpu=32
> [xry111@gcc400 ~]$ readelf -a a.out | grep FLOAT
>   Flags:                             0x43, DOUBLE-FLOAT, OBJ-v1
> 
> Why would "lp64s" and "fpu=32" lead to double float ?!
> 
> Cc += Weining, though I'm not sure if this issue still exists in LLVM
> mainline.
> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University


本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 
This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.