diff mbox series

RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS

Message ID 20231211083734.2548970-1-juzhe.zhong@rivai.ai
State Superseded
Headers
Series RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-lint success Lint passed
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gc-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv32gc_zba_zbb_zbc_zbs-ilp32d-non-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-non-multilib success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
rivoscibot/toolchain-ci-rivos-test success Testing passed

Commit Message

juzhe.zhong@rivai.ai Dec. 11, 2023, 8:37 a.m. UTC 
  Hi, Richard. This patch fixes an ICE on record_use during RTL_SSA initialization RISC-V backend VSETVL PASS.

This is the ICE:

0x11a8603 partial_subreg_p(machine_mode, machine_mode)
        ../../../../gcc/gcc/rtl.h:3187
0x3b695eb rtl_ssa::function_info::record_use(rtl_ssa::function_info::build_info&, rtl_ssa::insn_info*, rtx_obj_reference)
        ../../../../gcc/gcc/rtl-ssa/insns.cc:524

In record_use:

      if (HARD_REGISTER_NUM_P (regno)
	  && partial_subreg_p (use->mode (), mode))

Assertion failed on partial_subreg_p which is:

inline bool
partial_subreg_p (machine_mode outermode, machine_mode innermode)
{
  /* Modes involved in a subreg must be ordered.  In particular, we must
     always know at compile time whether the subreg is paradoxical.  */
  poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
  poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
  gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
  return maybe_lt (outer_prec, inner_prec);
}

RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).

The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.

(insn 168 173 170 31 (set (reg:RVVM1SI 101 v5 [311])
        (unspec:RVVM1SI [
                (unspec:V32BI [
                        (const_vector:V32BI [
                                (const_int 1 [0x1]) repeated x32
                            ])
                        (reg:DI 30 t5 [312])
                        (const_int 2 [0x2]) repeated x2
                        (reg:SI 66 vl)
                        (reg:SI 67 vtype)
                    ] UNSPEC_VPREDICATE)
                (unspec:RVVM1SI [
                        (reg:V32SI 96 v0 [orig:185 vect__96.40 ] [185])   -----> VLS mode NUNITS = 32 elements.
                        (reg:RVVM1SI 113 v17 [439])                       -----> VLA mode NUNITS = [8, 8] elements.
                    ] UNSPEC_REDUC_XOR)
                (unspec:RVVM1SI [
                        (reg:SI 0 zero)
                    ] UNSPEC_VUNDEF)
            ] UNSPEC_REDUC)) 15948 {pred_redxorv32si}

In this case, record_use is trying to check partial_subreg_p (use->mode (), mode) for RTX = (reg:V32SI 96 v0 [orig:185 vect__96.40 ] [185]).

use->mode () == V32SImode, wheras mode = RVVM1SImode. Then it ICE since they are !ordered_p.

In this situation, the use mode for such use should keep the original mode (V32SI), no need to compare VLSmode and VLAmode which one is bigger.

I am still trying to find a way to walk around this issue in RISC-V backend.

Is this patch fix ok for the trunk ? If not, could you give me a suggestion to walk around this ICE in RISC-V backend.

Thanks.

gcc/ChangeLog:

	* rtl-ssa/insns.cc (function_info::record_use): Add ordered_p.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/vsetvl/avl_use_bug-2.c: New test.

---
 gcc/rtl-ssa/insns.cc                          |  2 ++
 .../riscv/rvv/vsetvl/avl_use_bug-2.c          | 21 +++++++++++++++++++
 2 files changed, 23 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_use_bug-2.c

Comments

Robin Dapp Dec. 11, 2023, 9:01 a.m. UTC | #1 
> In record_use:
> 
>       if (HARD_REGISTER_NUM_P (regno)
> 	  && partial_subreg_p (use->mode (), mode))
> 
> Assertion failed on partial_subreg_p which is:
> 
> inline bool
> partial_subreg_p (machine_mode outermode, machine_mode innermode)
> {
>   /* Modes involved in a subreg must be ordered.  In particular, we must
>      always know at compile time whether the subreg is paradoxical.  */
>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>   return maybe_lt (outer_prec, inner_prec);
> }
> 
> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
> 
> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.

Maybe as additional context: The second input (which has a VLA mode here)
is not used entirely but just its first element.  This serves as initial
value for the reduction.

I'm not sure we'd want to model it as subreg here (endianness etc?).
Could we have a VLS-mode equivalent for the VLA mode that only holds
one element?

Regards
 Robin
juzhe.zhong@rivai.ai Dec. 11, 2023, 9:04 a.m. UTC | #2 
>> I'm not sure we'd want to model it as subreg here (endianness etc?).
>> Could we have a VLS-mode equivalent for the VLA mode that only holds
>> one element?

Yes. This is the last chance to walk around it here but we will end up with more patterns.
since reduction dest operand always LMUL = 1 mode.

So, when -march=rv64gcv, the dest mode should be V4SI, if -march=rv64gcv_zvl256b, the dest mode should be V8SI.
...etc.  Different TARGET_MIN_VLEN, different M1 mode. It's going to be a big change in RISC-V backend.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-12-11 17:01
To: Juzhe-Zhong; gcc-patches
CC: rdapp.gcc; richard.sandiford
Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
> In record_use:
> 
>       if (HARD_REGISTER_NUM_P (regno)
>   && partial_subreg_p (use->mode (), mode))
> 
> Assertion failed on partial_subreg_p which is:
> 
> inline bool
> partial_subreg_p (machine_mode outermode, machine_mode innermode)
> {
>   /* Modes involved in a subreg must be ordered.  In particular, we must
>      always know at compile time whether the subreg is paradoxical.  */
>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>   return maybe_lt (outer_prec, inner_prec);
> }
> 
> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
> 
> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
 
Maybe as additional context: The second input (which has a VLA mode here)
is not used entirely but just its first element.  This serves as initial
value for the reduction.
 
I'm not sure we'd want to model it as subreg here (endianness etc?).
Could we have a VLS-mode equivalent for the VLA mode that only holds
one element?
 
Regards
Robin
juzhe.zhong@rivai.ai Dec. 11, 2023, 9:10 a.m. UTC | #3 
I think it's reasonable refactor reduction instruction pattern work around this issue.

Going to send a patch to apply this solution.

So drop this patch.  Sorry for bothering Richard S.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-12-11 17:01
To: Juzhe-Zhong; gcc-patches
CC: rdapp.gcc; richard.sandiford
Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
> In record_use:
> 
>       if (HARD_REGISTER_NUM_P (regno)
>   && partial_subreg_p (use->mode (), mode))
> 
> Assertion failed on partial_subreg_p which is:
> 
> inline bool
> partial_subreg_p (machine_mode outermode, machine_mode innermode)
> {
>   /* Modes involved in a subreg must be ordered.  In particular, we must
>      always know at compile time whether the subreg is paradoxical.  */
>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>   return maybe_lt (outer_prec, inner_prec);
> }
> 
> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
> 
> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
 
Maybe as additional context: The second input (which has a VLA mode here)
is not used entirely but just its first element.  This serves as initial
value for the reduction.
 
I'm not sure we'd want to model it as subreg here (endianness etc?).
Could we have a VLS-mode equivalent for the VLA mode that only holds
one element?
 
Regards
Robin
Robin Dapp Dec. 11, 2023, 9:11 a.m. UTC | #4 
> Yes. This is the last chance to walk around it here but we will end up with more patterns.
> since reduction dest operand always LMUL = 1 mode.
> 
> So, when -march=rv64gcv, the dest mode should be V4SI, if -march=rv64gcv_zvl256b, the dest mode should be V8SI.
> ...etc.  Different TARGET_MIN_VLEN, different M1 mode. It's going to be a big change in RISC-V backend.

Hmm I haven't really thought this through yet (nor checked the spec
in detail) but isn't the result always a 1-element thing?  I.e. a
V1SI regardless of the input vlen?  That would also mean various
changes of course.

Regards
 Robin
juzhe.zhong@rivai.ai Dec. 11, 2023, 9:26 a.m. UTC | #5 
Oh. I just confirmed. V1SI make perfect sens since we never apply partial vectorization for VLSmode.

Drop this patch and going to refactor reduction pattern to fix this issue.

Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-12-11 17:11
To: juzhe.zhong@rivai.ai; gcc-patches
CC: rdapp.gcc; richard.sandiford
Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
> Yes. This is the last chance to walk around it here but we will end up with more patterns.
> since reduction dest operand always LMUL = 1 mode.
> 
> So, when -march=rv64gcv, the dest mode should be V4SI, if -march=rv64gcv_zvl256b, the dest mode should be V8SI.
> ...etc.  Different TARGET_MIN_VLEN, different M1 mode. It's going to be a big change in RISC-V backend.
 
Hmm I haven't really thought this through yet (nor checked the spec
in detail) but isn't the result always a 1-element thing?  I.e. a
V1SI regardless of the input vlen?  That would also mean various
changes of course.
 
Regards
Robin
Richard Sandiford Dec. 11, 2023, 11:45 a.m. UTC | #6 
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> I think it's reasonable refactor reduction instruction pattern work around this issue.
>
> Going to send a patch to apply this solution.
>
> So drop this patch.  Sorry for bothering Richard S.

It wasn't a bother.

On the patch: as things stand, we try to make the use_info's mode be
at least as big as all the uses in the instruction.  So if we did
want to handle unordered modes for hard registers, I think we would need
to fall back on the register's natural mode instead.  I did something
similar in LRA recently for PR112278 (6e2e0ce6795).

So if in future you would like to change RTL-SSA, a fix along those
lines would be fine with me.

Thanks,
Richard

>
>
>
> juzhe.zhong@rivai.ai
>  
> From: Robin Dapp
> Date: 2023-12-11 17:01
> To: Juzhe-Zhong; gcc-patches
> CC: rdapp.gcc; richard.sandiford
> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
>> In record_use:
>> 
>>       if (HARD_REGISTER_NUM_P (regno)
>>   && partial_subreg_p (use->mode (), mode))
>> 
>> Assertion failed on partial_subreg_p which is:
>> 
>> inline bool
>> partial_subreg_p (machine_mode outermode, machine_mode innermode)
>> {
>>   /* Modes involved in a subreg must be ordered.  In particular, we must
>>      always know at compile time whether the subreg is paradoxical.  */
>>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>>   return maybe_lt (outer_prec, inner_prec);
>> }
>> 
>> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
>> 
>> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
>  
> Maybe as additional context: The second input (which has a VLA mode here)
> is not used entirely but just its first element.  This serves as initial
> value for the reduction.
>  
> I'm not sure we'd want to model it as subreg here (endianness etc?).
> Could we have a VLS-mode equivalent for the VLA mode that only holds
> one element?
>  
> Regards
> Robin
>  
>
juzhe.zhong@rivai.ai Dec. 11, 2023, 11:54 a.m. UTC | #7 
Thanks Richard.

It would be great if you are ok I can fix it in RTL_SSA.
I leverage your LRA patch in RTL_SSA:

  else
    {
      // Record the mode of the largest use.  The choice is arbitrary if
      // the instruction (unusually) references the same register in two
      // different but equal-sized modes.
      gcc_checking_assert (use->insn () == insn);
      if (HARD_REGISTER_NUM_P (regno))
        {
          if (!ordered_p (GET_MODE_PRECISION (use->mode ()),
                          GET_MODE_PRECISION (mode)))
            use->set_mode (reg_raw_mode[regno]);
          else if (partial_subreg_p (use->mode (), mode))
            use->set_mode (mode);
        }
      use->record_reference (ref, false);
    }

Is it reasonable to you ?

Thanks.


juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-12-11 19:45
To: juzhe.zhong\@rivai.ai
CC: Robin Dapp; gcc-patches
Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> I think it's reasonable refactor reduction instruction pattern work around this issue.
>
> Going to send a patch to apply this solution.
>
> So drop this patch.  Sorry for bothering Richard S.
 
It wasn't a bother.
 
On the patch: as things stand, we try to make the use_info's mode be
at least as big as all the uses in the instruction.  So if we did
want to handle unordered modes for hard registers, I think we would need
to fall back on the register's natural mode instead.  I did something
similar in LRA recently for PR112278 (6e2e0ce6795).
 
So if in future you would like to change RTL-SSA, a fix along those
lines would be fine with me.
 
Thanks,
Richard
 
>
>
>
> juzhe.zhong@rivai.ai
>  
> From: Robin Dapp
> Date: 2023-12-11 17:01
> To: Juzhe-Zhong; gcc-patches
> CC: rdapp.gcc; richard.sandiford
> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
>> In record_use:
>> 
>>       if (HARD_REGISTER_NUM_P (regno)
>>   && partial_subreg_p (use->mode (), mode))
>> 
>> Assertion failed on partial_subreg_p which is:
>> 
>> inline bool
>> partial_subreg_p (machine_mode outermode, machine_mode innermode)
>> {
>>   /* Modes involved in a subreg must be ordered.  In particular, we must
>>      always know at compile time whether the subreg is paradoxical.  */
>>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>>   return maybe_lt (outer_prec, inner_prec);
>> }
>> 
>> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
>> 
>> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
>  
> Maybe as additional context: The second input (which has a VLA mode here)
> is not used entirely but just its first element.  This serves as initial
> value for the reduction.
>  
> I'm not sure we'd want to model it as subreg here (endianness etc?).
> Could we have a VLS-mode equivalent for the VLA mode that only holds
> one element?
>  
> Regards
> Robin
>  
>
Richard Sandiford Dec. 11, 2023, 12:12 p.m. UTC | #8 
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> Thanks Richard.
>
> It would be great if you are ok I can fix it in RTL_SSA.
> I leverage your LRA patch in RTL_SSA:
>
>   else
>     {
>       // Record the mode of the largest use.  The choice is arbitrary if
>       // the instruction (unusually) references the same register in two
>       // different but equal-sized modes.
>       gcc_checking_assert (use->insn () == insn);
>       if (HARD_REGISTER_NUM_P (regno))
>         {
>           if (!ordered_p (GET_MODE_PRECISION (use->mode ()),
>                           GET_MODE_PRECISION (mode)))
>             use->set_mode (reg_raw_mode[regno]);
>           else if (partial_subreg_p (use->mode (), mode))
>             use->set_mode (mode);
>         }
>       use->record_reference (ref, false);
>     }
>
> Is it reasonable to you ?

Yeah, the above is OK for trunk, thanks.

Richard

>
> Thanks.
>
>
> juzhe.zhong@rivai.ai
>  
> From: Richard Sandiford
> Date: 2023-12-11 19:45
> To: juzhe.zhong\@rivai.ai
> CC: Robin Dapp; gcc-patches
> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
> "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
>> I think it's reasonable refactor reduction instruction pattern work around this issue.
>>
>> Going to send a patch to apply this solution.
>>
>> So drop this patch.  Sorry for bothering Richard S.
>  
> It wasn't a bother.
>  
> On the patch: as things stand, we try to make the use_info's mode be
> at least as big as all the uses in the instruction.  So if we did
> want to handle unordered modes for hard registers, I think we would need
> to fall back on the register's natural mode instead.  I did something
> similar in LRA recently for PR112278 (6e2e0ce6795).
>  
> So if in future you would like to change RTL-SSA, a fix along those
> lines would be fine with me.
>  
> Thanks,
> Richard
>  
>>
>>
>>
>> juzhe.zhong@rivai.ai
>>  
>> From: Robin Dapp
>> Date: 2023-12-11 17:01
>> To: Juzhe-Zhong; gcc-patches
>> CC: rdapp.gcc; richard.sandiford
>> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
>>> In record_use:
>>> 
>>>       if (HARD_REGISTER_NUM_P (regno)
>>>   && partial_subreg_p (use->mode (), mode))
>>> 
>>> Assertion failed on partial_subreg_p which is:
>>> 
>>> inline bool
>>> partial_subreg_p (machine_mode outermode, machine_mode innermode)
>>> {
>>>   /* Modes involved in a subreg must be ordered.  In particular, we must
>>>      always know at compile time whether the subreg is paradoxical.  */
>>>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>>>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>>>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>>>   return maybe_lt (outer_prec, inner_prec);
>>> }
>>> 
>>> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
>>> 
>>> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
>>  
>> Maybe as additional context: The second input (which has a VLA mode here)
>> is not used entirely but just its first element.  This serves as initial
>> value for the reduction.
>>  
>> I'm not sure we'd want to model it as subreg here (endianness etc?).
>> Could we have a VLS-mode equivalent for the VLA mode that only holds
>> one element?
>>  
>> Regards
>> Robin
>>  
>>  
>
juzhe.zhong@rivai.ai Dec. 11, 2023, 12:19 p.m. UTC | #9 
Thanks Richard.

Committed with V2:
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640172.html 



juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-12-11 20:12
To: juzhe.zhong\@rivai.ai
CC: Robin Dapp; gcc-patches
Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> Thanks Richard.
>
> It would be great if you are ok I can fix it in RTL_SSA.
> I leverage your LRA patch in RTL_SSA:
>
>   else
>     {
>       // Record the mode of the largest use.  The choice is arbitrary if
>       // the instruction (unusually) references the same register in two
>       // different but equal-sized modes.
>       gcc_checking_assert (use->insn () == insn);
>       if (HARD_REGISTER_NUM_P (regno))
>         {
>           if (!ordered_p (GET_MODE_PRECISION (use->mode ()),
>                           GET_MODE_PRECISION (mode)))
>             use->set_mode (reg_raw_mode[regno]);
>           else if (partial_subreg_p (use->mode (), mode))
>             use->set_mode (mode);
>         }
>       use->record_reference (ref, false);
>     }
>
> Is it reasonable to you ?
 
Yeah, the above is OK for trunk, thanks.
 
Richard
 
>
> Thanks.
>
>
> juzhe.zhong@rivai.ai
>  
> From: Richard Sandiford
> Date: 2023-12-11 19:45
> To: juzhe.zhong\@rivai.ai
> CC: Robin Dapp; gcc-patches
> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
> "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
>> I think it's reasonable refactor reduction instruction pattern work around this issue.
>>
>> Going to send a patch to apply this solution.
>>
>> So drop this patch.  Sorry for bothering Richard S.
>  
> It wasn't a bother.
>  
> On the patch: as things stand, we try to make the use_info's mode be
> at least as big as all the uses in the instruction.  So if we did
> want to handle unordered modes for hard registers, I think we would need
> to fall back on the register's natural mode instead.  I did something
> similar in LRA recently for PR112278 (6e2e0ce6795).
>  
> So if in future you would like to change RTL-SSA, a fix along those
> lines would be fine with me.
>  
> Thanks,
> Richard
>  
>>
>>
>>
>> juzhe.zhong@rivai.ai
>>  
>> From: Robin Dapp
>> Date: 2023-12-11 17:01
>> To: Juzhe-Zhong; gcc-patches
>> CC: rdapp.gcc; richard.sandiford
>> Subject: Re: [PATCH] RTL-SSA: Fix ICE on record_use of RTL_SSA for RISC-V VSETVL PASS
>>> In record_use:
>>> 
>>>       if (HARD_REGISTER_NUM_P (regno)
>>>   && partial_subreg_p (use->mode (), mode))
>>> 
>>> Assertion failed on partial_subreg_p which is:
>>> 
>>> inline bool
>>> partial_subreg_p (machine_mode outermode, machine_mode innermode)
>>> {
>>>   /* Modes involved in a subreg must be ordered.  In particular, we must
>>>      always know at compile time whether the subreg is paradoxical.  */
>>>   poly_int64 outer_prec = GET_MODE_PRECISION (outermode);
>>>   poly_int64 inner_prec = GET_MODE_PRECISION (innermode);
>>>   gcc_checking_assert (ordered_p (outer_prec, inner_prec));                     -----> cause ICE.
>>>   return maybe_lt (outer_prec, inner_prec);
>>> }
>>> 
>>> RISC-V VSETVL PASS is an advanced lazy vsetvl insertion PASS after RA (register allocation).
>>> 
>>> The rootcause is that we have a pattern (reduction instruction) that includes both VLA (length-agnostic) and VLS (fixed-length) modes.
>>  
>> Maybe as additional context: The second input (which has a VLA mode here)
>> is not used entirely but just its first element.  This serves as initial
>> value for the reduction.
>>  
>> I'm not sure we'd want to model it as subreg here (endianness etc?).
>> Could we have a VLS-mode equivalent for the VLA mode that only holds
>> one element?
>>  
>> Regards
>> Robin
>>  
>>  
>
diff mbox series

Patch

diff --git a/gcc/rtl-ssa/insns.cc b/gcc/rtl-ssa/insns.cc
index 2fa48e0dacd..5b9c5baf8c7 100644
--- a/gcc/rtl-ssa/insns.cc
+++ b/gcc/rtl-ssa/insns.cc
@@ -521,6 +521,8 @@  function_info::record_use (build_info &bi, insn_info *insn,
       // different but equal-sized modes.
       gcc_checking_assert (use->insn () == insn);
       if (HARD_REGISTER_NUM_P (regno)
+	  && ordered_p (GET_MODE_PRECISION (use->mode ()),
+			GET_MODE_PRECISION (mode))
 	  && partial_subreg_p (use->mode (), mode))
 	use->set_mode (mode);
       use->record_reference (ref, false);
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_use_bug-2.c b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_use_bug-2.c
new file mode 100644
index 00000000000..bbc02eab818
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_use_bug-2.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64d --param=riscv-autovec-lmul=m4 -O3 -fomit-frame-pointer -funroll-loops" } */
+
+int safe_lshift_func_int32_t_s_s_left, safe_lshift_func_int32_t_s_s_right,
+    safe_sub_func_uint64_t_u_u_ui2, safe_mul_func_uint64_t_u_u_ui2, g_79_2,
+    g_97_l_439;
+void g_97(int * __restrict l_437)
+{
+  for (; g_97_l_439; g_97_l_439 += 1)
+    for (char l_502 = 0; l_502 < 4; l_502++)
+      {
+        int __trans_tmp_14 = ((safe_lshift_func_int32_t_s_s_right >= 2
+                               || safe_lshift_func_int32_t_s_s_left)
+                              ? 1 : safe_lshift_func_int32_t_s_s_right);
+        long __trans_tmp_15 = __trans_tmp_14 * safe_mul_func_uint64_t_u_u_ui2;
+        unsigned short __trans_tmp_16 = -__trans_tmp_15;
+        int __trans_tmp_7
+          = (__trans_tmp_16 ^ 65535UL) - safe_sub_func_uint64_t_u_u_ui2;
+        *l_437 ^= (short)(__trans_tmp_7 ^ g_79_2);
+      }
+}