public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently
@ 2023-03-03 3:55 xry111 at gcc dot gnu.org
2023-03-03 3:57 ` [Bug target/109000] " xry111 at gcc dot gnu.org
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-03-03 3:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109000
Bug ID: 109000
Summary: LoongArch: "unmatched" -mabi and -mfpu setting can
break ABI silently
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: xry111 at gcc dot gnu.org
Target Milestone: ---
$ cat t.c
double t(double x)
{
return 1.0 / x;
}
$ ~/gcc-trunk/bin/gcc t.c -O2 -c -mabi=lp64s
$ readelf -a t.o | grep FLOAT
Flags: 0x41, SOFT-FLOAT, OBJ-v1
$ objdump -d t.o
t.o: file format elf64-loongarch
Disassembly of section .text:
0000000000000000 <t>:
0: 01145800 frecip.d $fa0, $fa0
4: 4c000020 jirl $zero, $ra, 0
So we now have a SOFT-FLOAT object using FPR for arguments and return values
:(.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Bug target/109000] LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently
2023-03-03 3:55 [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently xry111 at gcc dot gnu.org
@ 2023-03-03 3:57 ` xry111 at gcc dot gnu.org
2023-03-06 7:56 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-03-03 3:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109000
Xi Ruoyao <xry111 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |12.2.0, 13.0
Last reconfirmed| |2023-03-03
Target| |loongarch64-linux-gnu
Assignee|unassigned at gcc dot gnu.org |xry111 at gcc dot gnu.org
Ever confirmed|0 |1
Keywords| |wrong-code
Status|UNCONFIRMED |ASSIGNED
Target Milestone|--- |12.3
--- Comment #1 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
I've already made the patch. Just create a PR to track the issue.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Bug target/109000] LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently
2023-03-03 3:55 [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently xry111 at gcc dot gnu.org
2023-03-03 3:57 ` [Bug target/109000] " xry111 at gcc dot gnu.org
@ 2023-03-06 7:56 ` cvs-commit at gcc dot gnu.org
2023-03-06 7:57 ` cvs-commit at gcc dot gnu.org
2023-03-06 7:58 ` xry111 at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-03-06 7:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109000
--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>:
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Bug target/109000] LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently
2023-03-03 3:55 [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently xry111 at gcc dot gnu.org
2023-03-03 3:57 ` [Bug target/109000] " xry111 at gcc dot gnu.org
2023-03-06 7:56 ` cvs-commit at gcc dot gnu.org
@ 2023-03-06 7:57 ` cvs-commit at gcc dot gnu.org
2023-03-06 7:58 ` xry111 at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-03-06 7:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109000
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>:
https://gcc.gnu.org/g:eb50091457a789e2f2a15c2e5d08e0d79d3195b1
commit r12-9225-geb50091457a789e2f2a15c2e5d08e0d79d3195b1
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.
(cherry picked from commit 75eccddef5784bc5e262af31f535267a9c4e993e)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Bug target/109000] LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently
2023-03-03 3:55 [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently xry111 at gcc dot gnu.org
` (2 preceding siblings ...)
2023-03-06 7:57 ` cvs-commit at gcc dot gnu.org
@ 2023-03-06 7:58 ` xry111 at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-03-06 7:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109000
Xi Ruoyao <xry111 at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #4 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
Fixed for trunk and gcc-12.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-06 7:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-03 3:55 [Bug target/109000] New: LoongArch: "unmatched" -mabi and -mfpu setting can break ABI silently xry111 at gcc dot gnu.org
2023-03-03 3:57 ` [Bug target/109000] " xry111 at gcc dot gnu.org
2023-03-06 7:56 ` cvs-commit at gcc dot gnu.org
2023-03-06 7:57 ` cvs-commit at gcc dot gnu.org
2023-03-06 7:58 ` xry111 at gcc dot gnu.org
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).