diff mbox series

riscv: Fix scope for memory model calculation

Message ID 20230606045130.1687824-2-dimitar@dinux.eu
State Committed
Commit 7f26e76c9848aeea9ec10ea701a6168464a4a9c2
Headers
Series riscv: Fix scope for memory model calculation |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed

Commit Message

Dimitar Dimitrov June 6, 2023, 4:51 a.m. UTC 
  During libgcc configure stage for riscv32-none-elf, when
"--enable-checking=yes,rtl" has been activated, the following error
is observed:

  configure:3814: /home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/xgcc -B/home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/bin/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/lib/ -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/include -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/sys-include    -c -g -O2  conftest.c >&5
  during RTL pass: final
  conftest.c: In function 'main':
  conftest.c:16:1: internal compiler error: RTL check: expected code 'const_int', have 'reg' in riscv_print_operand, at config/riscv/riscv.cc:4462
     16 | }
        | ^
  0x843c4d rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int, char const*)
          /mnt/nvme/dinux/local-workspace/gcc/gcc/rtl.cc:916
  0x8ea823 riscv_print_operand
          /mnt/nvme/dinux/local-workspace/gcc/gcc/config/riscv/riscv.cc:4462
  0xde84b5 output_operand(rtx_def*, int)
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3632
  0xde8ef8 output_asm_insn(char const*, rtx_def**)
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3544
  0xded33b output_asm_insn(char const*, rtx_def**)
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3421
  0xded33b final_scan_insn_1
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2841
  0xded6cb final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2887
  0xded8b7 final_1
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:1979
  0xdee518 rest_of_handle_final
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4240
  0xdee518 execute
          /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4318

Fix by moving the calculation of memmodel to the cases where it is used.

Regression tested for riscv32-none-elf. No changes in gcc.sum and
g++.sum.  I don't have setup to test riscv64.

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_print_operand): Calculate
	memmodel only when it is valid.

Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
---
 gcc/config/riscv/riscv.cc | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Jeff Law June 7, 2023, 2:38 a.m. UTC | #1 
On 6/5/23 22:51, Dimitar Dimitrov wrote:
> During libgcc configure stage for riscv32-none-elf, when
> "--enable-checking=yes,rtl" has been activated, the following error
> is observed:
> 
>    configure:3814: /home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/xgcc -B/home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/bin/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/lib/ -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/include -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/sys-include    -c -g -O2  conftest.c >&5
>    during RTL pass: final
>    conftest.c: In function 'main':
>    conftest.c:16:1: internal compiler error: RTL check: expected code 'const_int', have 'reg' in riscv_print_operand, at config/riscv/riscv.cc:4462
>       16 | }
>          | ^
>    0x843c4d rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int, char const*)
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/rtl.cc:916
>    0x8ea823 riscv_print_operand
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/config/riscv/riscv.cc:4462
>    0xde84b5 output_operand(rtx_def*, int)
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3632
>    0xde8ef8 output_asm_insn(char const*, rtx_def**)
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3544
>    0xded33b output_asm_insn(char const*, rtx_def**)
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3421
>    0xded33b final_scan_insn_1
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2841
>    0xded6cb final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2887
>    0xded8b7 final_1
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:1979
>    0xdee518 rest_of_handle_final
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4240
>    0xdee518 execute
>            /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4318
> 
> Fix by moving the calculation of memmodel to the cases where it is used.
> 
> Regression tested for riscv32-none-elf. No changes in gcc.sum and
> g++.sum.  I don't have setup to test riscv64.
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (riscv_print_operand): Calculate
> 	memmodel only when it is valid.
Good to see you poking around in the RISC-V world Dimitar!  Are you 
still poking at the PRU as well?

Anyway, this is fine for the trunk and for backporting to gcc-13 if the 
problem exists there as well.

jeff
Andrew Pinski June 7, 2023, 2:42 a.m. UTC | #2 
On Mon, Jun 5, 2023 at 9:52 PM Dimitar Dimitrov <dimitar@dinux.eu> wrote:
>
> During libgcc configure stage for riscv32-none-elf, when
> "--enable-checking=yes,rtl" has been activated, the following error
> is observed:
>
>   configure:3814: /home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/xgcc -B/home/dinux/projects/pru/local-workspace/riscv32-gcc-build/./gcc/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/bin/ -B/mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/lib/ -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/include -isystem /mnt/nvme/dinux/local-workspace/riscv32-opt/riscv32-none-elf/sys-include    -c -g -O2  conftest.c >&5
>   during RTL pass: final
>   conftest.c: In function 'main':
>   conftest.c:16:1: internal compiler error: RTL check: expected code 'const_int', have 'reg' in riscv_print_operand, at config/riscv/riscv.cc:4462

Note this is recorded as https://gcc.gnu.org/PR109725 .

Thanks,
Andrew Pinski

>      16 | }
>         | ^
>   0x843c4d rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int, char const*)
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/rtl.cc:916
>   0x8ea823 riscv_print_operand
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/config/riscv/riscv.cc:4462
>   0xde84b5 output_operand(rtx_def*, int)
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3632
>   0xde8ef8 output_asm_insn(char const*, rtx_def**)
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3544
>   0xded33b output_asm_insn(char const*, rtx_def**)
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:3421
>   0xded33b final_scan_insn_1
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2841
>   0xded6cb final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:2887
>   0xded8b7 final_1
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:1979
>   0xdee518 rest_of_handle_final
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4240
>   0xdee518 execute
>           /mnt/nvme/dinux/local-workspace/gcc/gcc/final.cc:4318
>
> Fix by moving the calculation of memmodel to the cases where it is used.
>
> Regression tested for riscv32-none-elf. No changes in gcc.sum and
> g++.sum.  I don't have setup to test riscv64.
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_print_operand): Calculate
>         memmodel only when it is valid.
>
> Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
> ---
>  gcc/config/riscv/riscv.cc | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index c15da1d0e30..fa4bc3e1f7e 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -4459,7 +4459,6 @@ riscv_print_operand (FILE *file, rtx op, int letter)
>      }
>    machine_mode mode = GET_MODE (op);
>    enum rtx_code code = GET_CODE (op);
> -  const enum memmodel model = memmodel_base (INTVAL (op));
>
>    switch (letter)
>      {
> @@ -4596,7 +4595,8 @@ riscv_print_operand (FILE *file, rtx op, int letter)
>        fputs (GET_RTX_NAME (code), file);
>        break;
>
> -    case 'A':
> +    case 'A': {
> +      const enum memmodel model = memmodel_base (INTVAL (op));
>        if (riscv_memmodel_needs_amo_acquire (model)
>           && riscv_memmodel_needs_amo_release (model))
>         fputs (".aqrl", file);
> @@ -4605,18 +4605,23 @@ riscv_print_operand (FILE *file, rtx op, int letter)
>        else if (riscv_memmodel_needs_amo_release (model))
>         fputs (".rl", file);
>        break;
> +    }
>
> -    case 'I':
> +    case 'I': {
> +      const enum memmodel model = memmodel_base (INTVAL (op));
>        if (model == MEMMODEL_SEQ_CST)
>         fputs (".aqrl", file);
>        else if (riscv_memmodel_needs_amo_acquire (model))
>         fputs (".aq", file);
>        break;
> +    }
>
> -    case 'J':
> +    case 'J': {
> +      const enum memmodel model = memmodel_base (INTVAL (op));
>        if (riscv_memmodel_needs_amo_release (model))
>         fputs (".rl", file);
>        break;
> +    }
>
>      case 'i':
>        if (code != REG)
> --
> 2.40.1
>
Dimitar Dimitrov June 7, 2023, 7:15 p.m. UTC | #3 
On Tue, Jun 06, 2023 at 08:38:14PM -0600, Jeff Law wrote:
> 
> 
> > Regression tested for riscv32-none-elf. No changes in gcc.sum and
> > g++.sum.  I don't have setup to test riscv64.
> > 
> > gcc/ChangeLog:
> > 
> > 	* config/riscv/riscv.cc (riscv_print_operand): Calculate
> > 	memmodel only when it is valid.
> Good to see you poking around in the RISC-V world Dimitar!  Are you still
> poking at the PRU as well?

Hi Jeff,

Yes, I'm still maintaining the PRU backend.

For this patch I was actually poking at the middle end, trying to
implement a small optimization for PRU (PR 106562).  And I wanted
to test if other targets would also benefit from it.

Thanks,
Dimitar

> 
> Anyway, this is fine for the trunk and for backporting to gcc-13 if the
> problem exists there as well.
> 
> jeff
Jeff Law June 7, 2023, 8:04 p.m. UTC | #4 
On 6/7/23 13:15, Dimitar Dimitrov wrote:
> On Tue, Jun 06, 2023 at 08:38:14PM -0600, Jeff Law wrote:
>>
>>
>>> Regression tested for riscv32-none-elf. No changes in gcc.sum and
>>> g++.sum.  I don't have setup to test riscv64.
>>>
>>> gcc/ChangeLog:
>>>
>>> 	* config/riscv/riscv.cc (riscv_print_operand): Calculate
>>> 	memmodel only when it is valid.
>> Good to see you poking around in the RISC-V world Dimitar!  Are you still
>> poking at the PRU as well?
> 
> Hi Jeff,
> 
> Yes, I'm still maintaining the PRU backend.
> 
> For this patch I was actually poking at the middle end, trying to
> implement a small optimization for PRU (PR 106562).  And I wanted
> to test if other targets would also benefit from it.
Ah!  Too bad, I'd love to have another engineer poking at RV stuff on a 
regular basis, but I'll take any cleanups/fixes/improvements you may 
have, of course!

RV32 isn't a bad test target though.  Certainly more modern than some of 
the ports you could have tested against.

Jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index c15da1d0e30..fa4bc3e1f7e 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -4459,7 +4459,6 @@  riscv_print_operand (FILE *file, rtx op, int letter)
     }
   machine_mode mode = GET_MODE (op);
   enum rtx_code code = GET_CODE (op);
-  const enum memmodel model = memmodel_base (INTVAL (op));
 
   switch (letter)
     {
@@ -4596,7 +4595,8 @@  riscv_print_operand (FILE *file, rtx op, int letter)
       fputs (GET_RTX_NAME (code), file);
       break;
 
-    case 'A':
+    case 'A': {
+      const enum memmodel model = memmodel_base (INTVAL (op));
       if (riscv_memmodel_needs_amo_acquire (model)
 	  && riscv_memmodel_needs_amo_release (model))
 	fputs (".aqrl", file);
@@ -4605,18 +4605,23 @@  riscv_print_operand (FILE *file, rtx op, int letter)
       else if (riscv_memmodel_needs_amo_release (model))
 	fputs (".rl", file);
       break;
+    }
 
-    case 'I':
+    case 'I': {
+      const enum memmodel model = memmodel_base (INTVAL (op));
       if (model == MEMMODEL_SEQ_CST)
 	fputs (".aqrl", file);
       else if (riscv_memmodel_needs_amo_acquire (model))
 	fputs (".aq", file);
       break;
+    }
 
-    case 'J':
+    case 'J': {
+      const enum memmodel model = memmodel_base (INTVAL (op));
       if (riscv_memmodel_needs_amo_release (model))
 	fputs (".rl", file);
       break;
+    }
 
     case 'i':
       if (code != REG)