public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Xi Ruoyao <xry111@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r13-6500] LoongArch: Stop -mfpu from silently breaking ABI [PR109000]
Date: Mon,  6 Mar 2023 07:56:21 +0000 (GMT)	[thread overview]
Message-ID: <20230306075621.0F9BC3858C2B@sourceware.org> (raw)

https://gcc.gnu.org/g:75eccddef5784bc5e262af31f535267a9c4e993e

commit r13-6500-g75eccddef5784bc5e262af31f535267a9c4e993e
Author: Xi Ruoyao <xry111@xry111.site>
Date:   Thu Mar 2 18:05:23 2023 +0800

    LoongArch: Stop -mfpu from silently breaking ABI [PR109000]
    
    In the toolchain convention, we describe -mfpu= as:
    
    "Selects the allowed set of basic floating-point instructions and
    registers. This option should not change the FP calling convention
    unless it's necessary."
    
    Though not explicitly stated, the rationale of this rule is to allow
    combinations like "-mabi=lp64s -mfpu=64".  This will be useful for
    running applications with LP64S/F ABI on a double-float-capable
    LoongArch hardware and using a math library with LP64S/F ABI but native
    double float HW instructions, for a better performance.
    
    And now a case in Linux kernel has again proven the usefulness of this
    kind of combination.  The AMDGPU DCN kernel driver needs to perform some
    floating-point operation, but the entire kernel uses LP64S ABI.  So the
    translation units of the AMDGPU DCN driver need to be compiled with
    -mfpu=64 (the kernel lacks soft-FP routines in libgcc), but -mabi=lp64s
    (or you can't link it with the other part of the kernel).
    
    Unfortunately, currently GCC uses TARGET_{HARD,SOFT,DOUBLE}_FLOAT to
    determine the floating calling convention.  This causes "-mfpu=64"
    silently allow using $fa* to pass parameters and return values EVEN IF
    -mabi=lp64s is used.  To make things worse, the generated object file
    has SOFT-FLOAT set in the eflags field so the linker will happily link
    it with other LP64S ABI object files, but obviously this will lead to
    bad results at runtime.  And for now all loongarch64 CPU models (-march
    settings) implies -mfpu=64 on by default, so the issue makes a single
    "-mabi=lp64s" option basically broken (fortunately most projects for eg
    the Linux kernel have used -msoft-float which implies both -mabi=lp64s
    and -mfpu=none as we've recommended in the toolchain convention doc).
    
    The fix is simple: use TARGET_*_FLOAT_ABI instead.
    
    I consider this a bug fix: the behavior difference from the toolchain
    convention doc is a bug, and generating object files with SOFT-FLOAT
    flag but parameters/return values passed through FPRs is definitely a
    bug.
    
    Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk and
    release/gcc-12 branch?
    
    gcc/ChangeLog:
    
            PR target/109000
            * config/loongarch/loongarch.h (FP_RETURN): Use
            TARGET_*_FLOAT_ABI instead of TARGET_*_FLOAT.
            (UNITS_PER_FP_ARG): Likewise.
    
    gcc/testsuite/ChangeLog:
    
            PR target/109000
            * gcc.target/loongarch/flt-abi-isa-1.c: New test.
            * gcc.target/loongarch/flt-abi-isa-2.c: New test.
            * gcc.target/loongarch/flt-abi-isa-3.c: New test.
            * gcc.target/loongarch/flt-abi-isa-4.c: New test.

Diff:
---
 gcc/config/loongarch/loongarch.h                   |  4 ++--
 gcc/testsuite/gcc.target/loongarch/flt-abi-isa-1.c | 14 ++++++++++++++
 gcc/testsuite/gcc.target/loongarch/flt-abi-isa-2.c | 10 ++++++++++
 gcc/testsuite/gcc.target/loongarch/flt-abi-isa-3.c |  9 +++++++++
 gcc/testsuite/gcc.target/loongarch/flt-abi-isa-4.c | 10 ++++++++++
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/gcc/config/loongarch/loongarch.h b/gcc/config/loongarch/loongarch.h
index f4e903d46bb..f8167875646 100644
--- a/gcc/config/loongarch/loongarch.h
+++ b/gcc/config/loongarch/loongarch.h
@@ -676,7 +676,7 @@ enum reg_class
    point values.  */
 
 #define GP_RETURN (GP_REG_FIRST + 4)
-#define FP_RETURN ((TARGET_SOFT_FLOAT) ? GP_RETURN : (FP_REG_FIRST + 0))
+#define FP_RETURN ((TARGET_SOFT_FLOAT_ABI) ? GP_RETURN : (FP_REG_FIRST + 0))
 
 #define MAX_ARGS_IN_REGISTERS 8
 
@@ -1154,6 +1154,6 @@ struct GTY (()) machine_function
 /* The largest type that can be passed in floating-point registers.  */
 /* TODO: according to mabi.  */
 #define UNITS_PER_FP_ARG  \
-  (TARGET_HARD_FLOAT ? (TARGET_DOUBLE_FLOAT ? 8 : 4) : 0)
+  (TARGET_HARD_FLOAT_ABI ? (TARGET_DOUBLE_FLOAT_ABI ? 8 : 4) : 0)
 
 #define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN)
diff --git a/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-1.c b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-1.c
new file mode 100644
index 00000000000..1c9490f6a87
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=lp64d -mfpu=64 -march=loongarch64 -O2" } */
+/* { dg-final { scan-assembler "frecip\\.d" } } */
+/* { dg-final { scan-assembler-not "movgr2fr\\.d" } } */
+/* { dg-final { scan-assembler-not "movfr2gr\\.d" } } */
+
+/* FPU is used for calculation and FPR is used for arguments and return
+   values.  */
+
+double
+t (double x)
+{
+  return 1.0 / x;
+}
diff --git a/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-2.c b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-2.c
new file mode 100644
index 00000000000..0580fd65d3a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=lp64s -mfpu=64 -march=loongarch64 -O2" } */
+/* { dg-final { scan-assembler "frecip\\.d" } } */
+/* { dg-final { scan-assembler "movgr2fr\\.d" } } */
+/* { dg-final { scan-assembler "movfr2gr\\.d" } } */
+
+/* FPU is used for calculation but FPR cannot be used for arguments and
+   return values.  */
+
+#include "flt-abi-isa-1.c"
diff --git a/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-3.c b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-3.c
new file mode 100644
index 00000000000..16a926f57a1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-3.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=lp64s -mfpu=none -march=loongarch64 -O2" } */
+/* { dg-final { scan-assembler-not "frecip\\.d" } } */
+/* { dg-final { scan-assembler-not "movgr2fr\\.d" } } */
+/* { dg-final { scan-assembler-not "movfr2gr\\.d" } } */
+
+/* FPU cannot be used at all.  */
+
+#include "flt-abi-isa-1.c"
diff --git a/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-4.c b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-4.c
new file mode 100644
index 00000000000..43b579c3fac
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/flt-abi-isa-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-msoft-float -march=loongarch64 -O2" } */
+/* { dg-final { scan-assembler-not "frecip\\.d" } } */
+/* { dg-final { scan-assembler-not "movgr2fr\\.d" } } */
+/* { dg-final { scan-assembler-not "movfr2gr\\.d" } } */
+
+/* -msoft-float implies both -mabi=lp64s and -mfpu=none.
+   FPU cannot be used at all.  */
+
+#include "flt-abi-isa-1.c"

                 reply	other threads:[~2023-03-06  7:56 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230306075621.0F9BC3858C2B@sourceware.org \
    --to=xry111@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).