[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #14 from Sebastian Huber --- Thanks for your help, it seems that this patch fixes the issue for RTEMS: diff --git a/gcc/config/rs6000/rtems.h b/gcc/config/rs6000/rtems.h index 57a2325991b..b36e64fec77 100644 --- a/gcc/config/rs6000/rtems.h +++ b/gcc/config/rs6000/rtems.h @@ -36,6 +36,10 @@ #endif #endif +/* RTEMS configured for the 32-bit multilibs doesn't support saving and + restoring 64-bit regs. */ +#define OS_MISSING_POWERPC64 !TARGET_64BIT + /* Copy and paste from linux64.h and freebsd64.h */ #undef TARGET_AIX #defineTARGET_AIX TARGET_64BIT
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #13 from Sebastian Huber --- (In reply to Kewen Lin from comment #12) > (In reply to Sebastian Huber from comment #10) > > (In reply to Kewen Lin from comment #9) > > > Note that now we only disable implicit powerpc64 for -m32 when the > > > OS_MISSING_POWERPC64 is set. > > > > > > /* Don't expect powerpc64 enabled on those OSes with > > > OS_MISSING_POWERPC64, > > > since they do not save and restore the high half of the GPRs > > > correctly > > > in all cases. If the user explicitly specifies it, we won't > > > interfere > > > with the user's specification. */ > > > #ifdef OS_MISSING_POWERPC64 > > > if (OS_MISSING_POWERPC64 > > > && TARGET_32BIT > > > && TARGET_POWERPC64 > > > && !(rs6000_isa_flags_explicit & OPTION_MASK_POWERPC64)) > > > rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; > > > #endif > > > > > > But rtems.h doesn't define OS_MISSING_POWERPC64 > > > > RTEMS supports the 64-bit PowerPC for the 64-bit multilibs. > > > > 64-bit kernel should support 64-bit PowerPC, but does 32-bit kernel support > saving and restoring 64-bit regs? For the 32-bit multilibs, we don't save/restore the full 64-bit registers. > > The current rtems.h is saying yes, if it's no, we should fix the rtems.h and > you won't need the explicit -mno-powerpc64 then. > > > btw, take the comments in freebsd64.h for example. > > /* FreeBSD doesn't support saving and restoring 64-bit regs with a 32-bit >kernel. This is supported when running on a 64-bit kernel with >COMPAT_FREEBSD32, but tell GCC it isn't so that our 32-bit binaries >are compatible. */ > #define OS_MISSING_POWERPC64 !TARGET_64BIT Thanks for the hint, I will try out this setting.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #12 from Kewen Lin --- (In reply to Sebastian Huber from comment #10) > (In reply to Kewen Lin from comment #9) > > Note that now we only disable implicit powerpc64 for -m32 when the > > OS_MISSING_POWERPC64 is set. > > > > /* Don't expect powerpc64 enabled on those OSes with OS_MISSING_POWERPC64, > > since they do not save and restore the high half of the GPRs correctly > > in all cases. If the user explicitly specifies it, we won't interfere > > with the user's specification. */ > > #ifdef OS_MISSING_POWERPC64 > > if (OS_MISSING_POWERPC64 > > && TARGET_32BIT > > && TARGET_POWERPC64 > > && !(rs6000_isa_flags_explicit & OPTION_MASK_POWERPC64)) > > rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; > > #endif > > > > But rtems.h doesn't define OS_MISSING_POWERPC64 > > RTEMS supports the 64-bit PowerPC for the 64-bit multilibs. > 64-bit kernel should support 64-bit PowerPC, but does 32-bit kernel support saving and restoring 64-bit regs? The current rtems.h is saying yes, if it's no, we should fix the rtems.h and you won't need the explicit -mno-powerpc64 then. btw, take the comments in freebsd64.h for example. /* FreeBSD doesn't support saving and restoring 64-bit regs with a 32-bit kernel. This is supported when running on a 64-bit kernel with COMPAT_FREEBSD32, but tell GCC it isn't so that our 32-bit binaries are compatible. */ #define OS_MISSING_POWERPC64 !TARGET_64BIT
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #11 from Kewen Lin --- (In reply to Sebastian Huber from comment #8) > Yes, it seems that -mcpu=e6500 -mno-powerpc64 yields the right code for the > attached test case (with or without the -m32). The default is -m32 I guess? :) > > I am now a bit confused what the purpose of the -m32 and -m64 options is. For -m32/-m64, the manual says: Generate code for 32-bit or 64-bit environments of Darwin and SVR4 targets (including GNU/Linux). The 32-bit environment sets int, long and pointer to 32 bits and generates code that runs on any PowerPC variant. The 64-bit environment sets int to 32 bits and long and pointer to 64 bits, and generates code for PowerPC64, as for -mpowerpc64. But it's possible to interact with option powerpc64, like cpu e6500 which by default supports powerpc64 and if applied OS is able to support the necessary context switches, we want -mpowerpc64 kept and it's able to generate more efficient code (leveraging insns guarded with powerpc64 flag).
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #10 from Sebastian Huber --- (In reply to Kewen Lin from comment #9) > Note that now we only disable implicit powerpc64 for -m32 when the > OS_MISSING_POWERPC64 is set. > > /* Don't expect powerpc64 enabled on those OSes with OS_MISSING_POWERPC64, > since they do not save and restore the high half of the GPRs correctly > in all cases. If the user explicitly specifies it, we won't interfere > with the user's specification. */ > #ifdef OS_MISSING_POWERPC64 > if (OS_MISSING_POWERPC64 > && TARGET_32BIT > && TARGET_POWERPC64 > && !(rs6000_isa_flags_explicit & OPTION_MASK_POWERPC64)) > rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; > #endif > > But rtems.h doesn't define OS_MISSING_POWERPC64 RTEMS supports the 64-bit PowerPC for the 64-bit multilibs. > > gcc/config/rs6000/linux.h:#define OS_MISSING_POWERPC64 1 > gcc/config/rs6000/freebsd64.h:#define OS_MISSING_POWERPC64 !TARGET_64BIT > gcc/config/rs6000/aix.h:#define OS_MISSING_POWERPC64 1 > gcc/config/rs6000/linux64.h:#define OS_MISSING_POWERPC64 !TARGET_64BIT > > meanwhile cpu "e6500" has MASK_POWERPC64 set by default (it's 64bit core). > > That's why you still have powerpc64 flag set when you specify -m32 on rtems. For some applications, you don't need the 64-bit support on the e6500 machines. So, we have 32-bit and 64-bit multilibs. This is just a performance optimization for some applications.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #9 from Kewen Lin --- Note that now we only disable implicit powerpc64 for -m32 when the OS_MISSING_POWERPC64 is set. /* Don't expect powerpc64 enabled on those OSes with OS_MISSING_POWERPC64, since they do not save and restore the high half of the GPRs correctly in all cases. If the user explicitly specifies it, we won't interfere with the user's specification. */ #ifdef OS_MISSING_POWERPC64 if (OS_MISSING_POWERPC64 && TARGET_32BIT && TARGET_POWERPC64 && !(rs6000_isa_flags_explicit & OPTION_MASK_POWERPC64)) rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; #endif But rtems.h doesn't define OS_MISSING_POWERPC64 gcc/config/rs6000/linux.h:#define OS_MISSING_POWERPC64 1 gcc/config/rs6000/freebsd64.h:#define OS_MISSING_POWERPC64 !TARGET_64BIT gcc/config/rs6000/aix.h:#define OS_MISSING_POWERPC64 1 gcc/config/rs6000/linux64.h:#define OS_MISSING_POWERPC64 !TARGET_64BIT meanwhile cpu "e6500" has MASK_POWERPC64 set by default (it's 64bit core). That's why you still have powerpc64 flag set when you specify -m32 on rtems.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #8 from Sebastian Huber --- Yes, it seems that -mcpu=e6500 -mno-powerpc64 yields the right code for the attached test case (with or without the -m32). I am now a bit confused what the purpose of the -m32 and -m64 options is.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680
--- Comment #7 from Kewen Lin ---
(In reply to Sebastian Huber from comment #6)
> It seems that the change
>
> commit acc727cf02a1446dc00f8772f3f479fa3a508f8e
> Author: Kewen Lin
> Date: Tue Dec 27 04:13:07 2022 -0600
>
> rs6000: Rework option -mpowerpc64 handling [PR106680]
>
> causes a regression for -mcpu=e6500 -m32, for example:
>
> gcc -fpreprocessed -O2 -S -mcpu=e6500 -m32 -S imfs_add_node.c.67.s
> imfs_add_node.c.67.i
>
> diff -u imfs_add_node.c.67.s.good.e2acff49fb2962b921bf8b73984b89878b61492c
> imfs_add_node.c.67.s.bad.acc727cf02a1446dc00f8772f3f479fa3a508f8e
> --- imfs_add_node.c.67.s.good.e2acff49fb2962b921bf8b73984b89878b61492c
> 2024-01-20 12:15:15.143182571 +0100
> +++ imfs_add_node.c.67.s.bad.acc727cf02a1446dc00f8772f3f479fa3a508f8e
> 2024-01-20 12:11:46.804204927 +0100
> @@ -52,8 +52,8 @@
> bne- 0,.L4
> .L2:
> mr 4,29
> - addi 3,1,8
> li 5,24
> + addi 3,1,8
> bl rtems_filesystem_eval_path_start
> lis 9,IMFS_node_clone@ha
> lwz 10,20(3)
> @@ -63,12 +63,12 @@
> cmpw 0,10,9
> beq- 0,.L24
> li 4,134
> - addi 3,1,8
> + li 3,0
> bl rtems_filesystem_eval_path_error
> .L9:
> li 31,-1
> .L10:
> - addi 3,1,8
> + li 3,0
> bl rtems_filesystem_eval_path_cleanup
> .L1:
> lwz 0,116(1)
> @@ -93,7 +93,7 @@
> lwz 9,12(31)
> li 8,96
> lhz 10,16(31)
> - addi 3,1,8
> + li 3,0
> stw 8,24(1)
> stw 9,8(1)
> stw 10,12(1)
> @@ -105,7 +105,7 @@
> cmpwi 0,9,0
> beq- 0,.L9
> li 4,22
> - addi 3,1,8
> + li 3,0
> bl rtems_filesystem_eval_path_error
> b .L9
> .p2align 4,,15
> @@ -129,12 +129,9 @@
> stw 9,0(10)
> stw 10,4(9)
> bl _Timecounter_Getbintime
> - lwz 10,64(1)
> - lwz 11,68(1)
> - stw 10,40(30)
> - stw 11,44(30)
> - stw 10,48(30)
> - stw 11,52(30)
> + ld 9,64(1)
> + std 9,40(30)
> + std 9,48(30)
> b .L10
> .cfi_endproc
> .LFE351:
>
> For the call to rtems_filesystem_eval_path_cleanup() the register 3 should
> point to a structure on the stack. Correct is:
>
> - addi 3,1,8
>
> Wrong is:
>
> + li 3,0
>
> It seems that for the -mcpu=e6500 the -m32 option has not the right effect
> and some 64-bit instructions are generated, for example ld and std plus the
As the commit log, the previous behavior that -m32 also disables -mpowerpc64 is
wrong, -m{no,}powerpc64 should be independent of -m32/-m64.
> wrong function parameters.
I supposed that the behavior you wanted with -m32 is not to enable powerpc64
(since the previous behavior is -m32 can disable -mpowerpc64 as well), so I
think you can get the previous behavior if you specify one explicit
-mno-powerpc64 when adopting -m32.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 --- Comment #6 from Sebastian Huber --- It seems that the change commit acc727cf02a1446dc00f8772f3f479fa3a508f8e Author: Kewen Lin Date: Tue Dec 27 04:13:07 2022 -0600 rs6000: Rework option -mpowerpc64 handling [PR106680] causes a regression for -mcpu=e6500 -m32, for example: gcc -fpreprocessed -O2 -S -mcpu=e6500 -m32 -S imfs_add_node.c.67.s imfs_add_node.c.67.i diff -u imfs_add_node.c.67.s.good.e2acff49fb2962b921bf8b73984b89878b61492c imfs_add_node.c.67.s.bad.acc727cf02a1446dc00f8772f3f479fa3a508f8e --- imfs_add_node.c.67.s.good.e2acff49fb2962b921bf8b73984b89878b61492c 2024-01-20 12:15:15.143182571 +0100 +++ imfs_add_node.c.67.s.bad.acc727cf02a1446dc00f8772f3f479fa3a508f8e 2024-01-20 12:11:46.804204927 +0100 @@ -52,8 +52,8 @@ bne- 0,.L4 .L2: mr 4,29 - addi 3,1,8 li 5,24 + addi 3,1,8 bl rtems_filesystem_eval_path_start lis 9,IMFS_node_clone@ha lwz 10,20(3) @@ -63,12 +63,12 @@ cmpw 0,10,9 beq- 0,.L24 li 4,134 - addi 3,1,8 + li 3,0 bl rtems_filesystem_eval_path_error .L9: li 31,-1 .L10: - addi 3,1,8 + li 3,0 bl rtems_filesystem_eval_path_cleanup .L1: lwz 0,116(1) @@ -93,7 +93,7 @@ lwz 9,12(31) li 8,96 lhz 10,16(31) - addi 3,1,8 + li 3,0 stw 8,24(1) stw 9,8(1) stw 10,12(1) @@ -105,7 +105,7 @@ cmpwi 0,9,0 beq- 0,.L9 li 4,22 - addi 3,1,8 + li 3,0 bl rtems_filesystem_eval_path_error b .L9 .p2align 4,,15 @@ -129,12 +129,9 @@ stw 9,0(10) stw 10,4(9) bl _Timecounter_Getbintime - lwz 10,64(1) - lwz 11,68(1) - stw 10,40(30) - stw 11,44(30) - stw 10,48(30) - stw 11,52(30) + ld 9,64(1) + std 9,40(30) + std 9,48(30) b .L10 .cfi_endproc .LFE351: For the call to rtems_filesystem_eval_path_cleanup() the register 3 should point to a structure on the stack. Correct is: - addi 3,1,8 Wrong is: + li 3,0 It seems that for the -mcpu=e6500 the -m32 option has not the right effect and some 64-bit instructions are generated, for example ld and std plus the wrong function parameters.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 Sebastian Huber changed: What|Removed |Added CC||sebastian.huber@embedded-br ||ains.de --- Comment #5 from Sebastian Huber --- Created attachment 57174 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57174=edit Proprocessed test file.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 Kewen Lin changed: What|Removed |Added Resolution|--- |FIXED Target Milestone|--- |13.0 Status|ASSIGNED|RESOLVED --- Comment #4 from Kewen Lin --- Should be fixed on trunk.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680
--- Comment #3 from CVS Commits ---
The master branch has been updated by Kewen Lin :
https://gcc.gnu.org/g:acc727cf02a1446dc00f8772f3f479fa3a508f8e
commit r13-4894-gacc727cf02a1446dc00f8772f3f479fa3a508f8e
Author: Kewen Lin
Date: Tue Dec 27 04:13:07 2022 -0600
rs6000: Rework option -mpowerpc64 handling [PR106680]
PR106680 shows that -m32 -mpowerpc64 is different from
-mpowerpc64 -m32, this is determined by the way how we
handle option powerpc64 in rs6000_handle_option.
Segher pointed out this difference should be taken as
a bug and we should ensure that option powerpc64 is
independent of -m32/-m64. So this patch removes the
handlings in rs6000_handle_option and add some necessary
supports in rs6000_option_override_internal instead.
With this patch, if users specify -m{no-,}powerpc64, the
specified value is honoured, otherwise, for 64bit it
always enables OPTION_MASK_POWERPC64; while for 32bit
and TARGET_POWERPC64 and OS_MISSING_POWERPC64, it disables
OPTION_MASK_POWERPC64.
btw, following Segher's suggestion, I did some tries to warn
when OPTION_MASK_POWERPC64 is set for OS_MISSING_POWERPC64.
If warn for the case that powerpc64 is specified explicitly,
there are some TCs using -m32 -mpowerpc64 on ppc64-linux,
they need some updates, meanwhile the artificial run
with "--target_board=unix'{-m32/-mpowerpc64}'" will have
noisy warnings on ppc64-linux. If warn for the case that
it's specified implicitly, they can just be initialized by
TARGET_DEFAULT (like -m32 on ppc64-linux) or set from the
given cpu mask, we have to special case them and not to warn.
As Segher's latest comment, I decide not to warn them and
keep it consistent with before.
Bootstrapped and regress-tested on:
- powerpc64-linux-gnu P7 and P8 {-m64,-m32}
- powerpc64le-linux-gnu P9 and P10
- powerpc-ibm-aix7.2.0.0 {-maix64,-maix32}
- powerpc-darwin9 (with Iain's help)
PR target/106680
gcc/ChangeLog:
* common/config/rs6000/rs6000-common.cc (rs6000_handle_option):
Remove
the adjustment for option powerpc64 in -m64 handling, and remove
the
whole -m32 handling.
* config/rs6000/rs6000.cc (rs6000_option_override_internal): When
no
explicit powerpc64 option is provided, enable it for -m64. For 32
bit
and OS_MISSING_POWERPC64, disable powerpc64 if it's enabled but not
specified explicitly.
gcc/testsuite/ChangeLog:
* gcc.target/powerpc/pr106680-1.c: New test.
* gcc.target/powerpc/pr106680-2.c: New test.
* gcc.target/powerpc/pr106680-3.c: New test.
* gcc.target/powerpc/pr106680-4.c: New test.
2022-12-27 Kewen Lin
Iain Sandoe
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680 Kewen Lin changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Ever confirmed|0 |1 Assignee|unassigned at gcc dot gnu.org |linkw at gcc dot gnu.org Last reconfirmed||2022-08-29 --- Comment #2 from Kewen Lin --- Confirmed, I can reproduce it with cfarm machine gcc110, the issue is exactly like what its comment describes. I have a patch.
[Bug testsuite/106680] Test gcc.target/powerpc/bswap64-4.c fails on 32-bit BE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106680
Kewen Lin changed:
What|Removed |Added
CC||linkw at gcc dot gnu.org
--- Comment #1 from Kewen Lin ---
The related insns requires TARGET_POWERPC64, they are still available on 32bit
if the option -O2 -mpowerpc64 comes after -m32.
I think it suffers the issue as its comments:
/* On some versions of dejagnu this test will fail when biarch testing
with RUNTESTFLAGS="--target_board=unix'{-m64,-m32}'" due to -m32
being added on the command line after the dg-options -mpowerpc64.
common/config/rs6000/rs6000-common.c:rs6000_handle_option disables
-mpowerpc64 for -m32. */
Hi Mike,
Could you share which test box you used for testing? Or dejagnu version?
