Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Mon, Feb 5, 2024 at 3:56 PM Jeff Law wrote: > > > > On 2/5/24 05:00, Christoph Müllner wrote: > > On Sat, Feb 3, 2024 at 2:11 PM Andreas Schwab > > wrote: > >> > >> On Jan 30 2024, Christoph Müllner wrote: > >> > >>> retested > >> > >> Nope. > > > > Sorry for this. I tested for no regressions in the test suite with a > > cross-build and QEMU and did not do a Werror bootstrap build. I'll > > provide a fix for this later today (also breaking the line as it is > > longer than needed). > Right. And that's pretty standard given the state of the RISC-V > platforms. We've got a platform here that can bootstrap in a reasonable > amount of time, but I haven't set that up in the CI system yet. > > Until such systems are common, these niggling issues are bound to show up. > > It's just whitespace around the HOST_WIDE_INT_PRINT_DEC and wrapping the > long line, right? I've got that in my tree that's bootstrapping now. I > don't mind committing it later today. But if you get to it before my > bootstrap is done, feel free to commit as pre-approved. Pushed: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=184978cd74f962712e813030d58edc109ad9a92d > > jeff
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Feb 05 2024, Jeff Law wrote: > We're all aware you *can* do that. But it's never been a requirement to > commit a patch. It has always been a requirement that a patch does not break bootstrap. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On 2/5/24 08:08, Andreas Schwab wrote: On Feb 05 2024, Jeff Law wrote: Until such systems are common, these niggling issues are bound to show up. It won't if you do it properly: build with a cross compiler that was built from the same source and enable -Werror. We're all aware you *can* do that. But it's never been a requirement to commit a patch. jeff
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Feb 05 2024, Jeff Law wrote: > Until such systems are common, these niggling issues are bound to show up. It won't if you do it properly: build with a cross compiler that was built from the same source and enable -Werror. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On 2/5/24 05:00, Christoph Müllner wrote: On Sat, Feb 3, 2024 at 2:11 PM Andreas Schwab wrote: On Jan 30 2024, Christoph Müllner wrote: retested Nope. Sorry for this. I tested for no regressions in the test suite with a cross-build and QEMU and did not do a Werror bootstrap build. I'll provide a fix for this later today (also breaking the line as it is longer than needed). Right. And that's pretty standard given the state of the RISC-V platforms. We've got a platform here that can bootstrap in a reasonable amount of time, but I haven't set that up in the CI system yet. Until such systems are common, these niggling issues are bound to show up. It's just whitespace around the HOST_WIDE_INT_PRINT_DEC and wrapping the long line, right? I've got that in my tree that's bootstrapping now. I don't mind committing it later today. But if you get to it before my bootstrap is done, feel free to commit as pre-approved. jeff
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Sat, Feb 3, 2024 at 2:11 PM Andreas Schwab wrote: > > On Jan 30 2024, Christoph Müllner wrote: > > > retested > > Nope. Sorry for this. I tested for no regressions in the test suite with a cross-build and QEMU and did not do a Werror bootstrap build. I'll provide a fix for this later today (also breaking the line as it is longer than needed). > > ../../gcc/config/riscv/thead.cc:1144:22: error: invalid suffix on literal; > C++11 requires a space between literal and string macro > [-Werror=literal-suffix] > 1144 | fprintf (file, "(%s),"HOST_WIDE_INT_PRINT_DEC",%u", > reg_names[REGNO (addr.reg)], > | ^ > cc1plus: all warnings being treated as errors > make[3]: *** [../../gcc/config/riscv/t-riscv:127: thead.o] Error 1 > > -- > Andreas Schwab, sch...@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Jan 30 2024, Christoph Müllner wrote: > retested Nope. ../../gcc/config/riscv/thead.cc:1144:22: error: invalid suffix on literal; C++11 requires a space between literal and string macro [-Werror=literal-suffix] 1144 | fprintf (file, "(%s),"HOST_WIDE_INT_PRINT_DEC",%u", reg_names[REGNO (addr.reg)], | ^ cc1plus: all warnings being treated as errors make[3]: *** [../../gcc/config/riscv/t-riscv:127: thead.o] Error 1 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
On Mon, Jan 29, 2024 at 1:32 PM Kito Cheng wrote: > > LGTM I've rebased, retested (rv64+rv32) and merged this patch. Thanks! > > Jin Ma 於 2024年1月29日 週一 17:57 寫道： >> >> When using '%ld' to print 'long long int' variable, 'fprintf' will >> produce messy output on a 32-bit system, in an incorrect instruction >> being generated, such as 'th.lwib a1,(a0),-16,4294967295'. And the >> following error occurred during compilation: >> >> Assembler messages: >> Error: improper immediate value (18446744073709551615) >> >> gcc/ChangeLog: >> >> * config/riscv/thead.cc (th_print_operand_address): Change %ld >> to %lld. >> --- >> gcc/config/riscv/thead.cc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc >> index 2955bc5f8a9..e4b8c37bc28 100644 >> --- a/gcc/config/riscv/thead.cc >> +++ b/gcc/config/riscv/thead.cc >> @@ -1141,7 +1141,7 @@ th_print_operand_address (FILE *file, machine_mode >> mode, rtx x) >>return true; >> >> case ADDRESS_REG_WB: >> - fprintf (file, "(%s),%ld,%u", reg_names[REGNO (addr.reg)], >> + fprintf (file, "(%s),"HOST_WIDE_INT_PRINT_DEC",%u", reg_names[REGNO >> (addr.reg)], >>INTVAL (addr.offset) >> addr.shift, addr.shift); >> return true; >> >> -- >> 2.17.1 >>
Re: [PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
LGTM Jin Ma 於 2024年1月29日 週一 17:57 寫道： > When using '%ld' to print 'long long int' variable, 'fprintf' will > produce messy output on a 32-bit system, in an incorrect instruction > being generated, such as 'th.lwib a1,(a0),-16,4294967295'. And the > following error occurred during compilation: > > Assembler messages: > Error: improper immediate value (18446744073709551615) > > gcc/ChangeLog: > > * config/riscv/thead.cc (th_print_operand_address): Change %ld > to %lld. > --- > gcc/config/riscv/thead.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc > index 2955bc5f8a9..e4b8c37bc28 100644 > --- a/gcc/config/riscv/thead.cc > +++ b/gcc/config/riscv/thead.cc > @@ -1141,7 +1141,7 @@ th_print_operand_address (FILE *file, machine_mode > mode, rtx x) >return true; > > case ADDRESS_REG_WB: > - fprintf (file, "(%s),%ld,%u", reg_names[REGNO (addr.reg)], > + fprintf (file, "(%s),"HOST_WIDE_INT_PRINT_DEC",%u", reg_names[REGNO > (addr.reg)], >INTVAL (addr.offset) >> addr.shift, addr.shift); > return true; > > -- > 2.17.1 > >
[PATCH v2] RISC-V: THEAD: Fix improper immediate value for MODIFY_DISP instruction on 32-bit systems.
When using '%ld' to print 'long long int' variable, 'fprintf' will produce messy output on a 32-bit system, in an incorrect instruction being generated, such as 'th.lwib a1,(a0),-16,4294967295'. And the following error occurred during compilation: Assembler messages: Error: improper immediate value (18446744073709551615) gcc/ChangeLog: * config/riscv/thead.cc (th_print_operand_address): Change %ld to %lld. --- gcc/config/riscv/thead.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc index 2955bc5f8a9..e4b8c37bc28 100644 --- a/gcc/config/riscv/thead.cc +++ b/gcc/config/riscv/thead.cc @@ -1141,7 +1141,7 @@ th_print_operand_address (FILE *file, machine_mode mode, rtx x) return true; case ADDRESS_REG_WB: - fprintf (file, "(%s),%ld,%u", reg_names[REGNO (addr.reg)], + fprintf (file, "(%s),"HOST_WIDE_INT_PRINT_DEC",%u", reg_names[REGNO (addr.reg)], INTVAL (addr.offset) >> addr.shift, addr.shift); return true; -- 2.17.1
