RISC-V: Fix AVL/VL get ICE[VSETVL PASS]
Checks
Commit Message
Fix bunch of ICE in "vect" testsuite:
FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault)
FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors)
FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault)
FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors)
FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors)
gcc/ChangeLog:
* config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function.
(pass_vsetvl::compute_local_properties): Fix bug.
(pass_vsetvl::commit_vsetvls): Ditto.
* config/riscv/riscv-vsetvl.h: New function.
---
gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++-----------
gcc/config/riscv/riscv-vsetvl.h | 1 +
2 files changed, 31 insertions(+), 16 deletions(-)
Comments
Assuming prev is vsetvli instruction is kind of a strong assumption,
but it is guarded with gcc_assert, so it is a reasonable fix to me,
LGTM :)
On Tue, Aug 29, 2023 at 10:37 AM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> Fix bunch of ICE in "vect" testsuite:
> FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault)
> FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors)
> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors)
> FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault)
> FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors)
> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors)
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function.
> (pass_vsetvl::compute_local_properties): Fix bug.
> (pass_vsetvl::commit_vsetvls): Ditto.
> * config/riscv/riscv-vsetvl.h: New function.
>
> ---
> gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++-----------
> gcc/config/riscv/riscv-vsetvl.h | 1 +
> 2 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index f7ae6c16bee..73d672b083b 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info,
> return new_info;
> }
>
> +/* Wrapper helps to return the AVL or VL operand for the
> + vector_insn_info. Return AVL if the AVL is not VLMAX.
> + Otherwise, return the VL operand. */
> +rtx
> +vector_insn_info::get_avl_or_vl_reg (void) const
> +{
> + gcc_assert (has_avl_reg ());
> + if (!vlmax_avl_p (get_avl ()))
> + return get_avl ();
> +
> + if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ()))
> + return ::get_vl (get_insn ()->rtl ());
> +
> + if (get_avl_source ())
> + return get_avl_reg_rtx ();
> +
> + /* A DIRTY (polluted EMPTY) block if:
> + - get_insn is scalar move (no AVL or VL operand).
> + - get_avl_source is null (no def in the current DIRTY block).
> + Then we trace the previous insn which must be the insn
> + already inserted in Phase 2 to get the VL operand for VLMAX. */
> + rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ());
> + gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn));
> + return ::get_vl (prev_rinsn);
> +}
> +
> bool
> vector_insn_info::update_fault_first_load_avl (insn_info *insn)
> {
> @@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void)
> bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i);
> else if (expr->has_avl_reg ())
> {
> - rtx avl = vlmax_avl_p (expr->get_avl ())
> - ? get_vl (expr->get_insn ()->rtl ())
> - : expr->get_avl ();
> + rtx reg = expr->get_avl_or_vl_reg ();
> for (const insn_info *insn : bb->real_nondebug_insns ())
> {
> - if (find_access (insn->defs (), REGNO (avl)))
> + if (find_access (insn->defs (), REGNO (reg)))
> {
> bitmap_clear_bit (
> m_vector_manager->vector_transp[curr_bb_idx], i);
> break;
> }
> else if (vlmax_avl_p (expr->get_avl ())
> - && find_access (insn->uses (), REGNO (avl)))
> + && find_access (insn->uses (), REGNO (reg)))
> {
> bitmap_clear_bit (
> m_vector_manager->vector_transp[curr_bb_idx], i);
> @@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void)
> = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX);
> else if (vlmax_avl_p (reaching_out.get_avl ()))
> {
> - rtx vl = NULL_RTX;
> - /* For user VSETVL VL, AVL. We need to use VL operand here, so we
> - don't directly use get_avl_reg_rtx (). Instead, we use the VL
> - of the INSN->RTL (). */
> - if (!reaching_out.get_avl_source ())
> - {
> - gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ()));
> - vl = get_vl (reaching_out.get_insn ()->rtl ());
> - }
> - else
> - vl = reaching_out.get_avl_reg_rtx ();
> + rtx vl = reaching_out.get_avl_or_vl_reg ();
> new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl);
> }
> else
> diff --git a/gcc/config/riscv/riscv-vsetvl.h b/gcc/config/riscv/riscv-vsetvl.h
> index 4b5825d7f6b..2a315e45f31 100644
> --- a/gcc/config/riscv/riscv-vsetvl.h
> +++ b/gcc/config/riscv/riscv-vsetvl.h
> @@ -335,6 +335,7 @@ public:
>
> rtl_ssa::insn_info *get_insn () const { return m_insn; }
> const bool *get_demands (void) const { return m_demands; }
> + rtx get_avl_or_vl_reg (void) const;
> rtx get_avl_reg_rtx (void) const
> {
> return gen_rtx_REG (Pmode, get_avl_source ()->regno ());
> --
> 2.36.3
>
Committed, thanks Kito.
On 2023/8/29 10:46, Kito Cheng via Gcc-patches wrote:
> Assuming prev is vsetvli instruction is kind of a strong assumption,
> but it is guarded with gcc_assert, so it is a reasonable fix to me,
> LGTM :)
>
> On Tue, Aug 29, 2023 at 10:37 AM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>>
>> Fix bunch of ICE in "vect" testsuite:
>> FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault)
>> FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors)
>> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
>> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors)
>> FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault)
>> FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors)
>> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault)
>> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors)
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function.
>> (pass_vsetvl::compute_local_properties): Fix bug.
>> (pass_vsetvl::commit_vsetvls): Ditto.
>> * config/riscv/riscv-vsetvl.h: New function.
>>
>> ---
>> gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++-----------
>> gcc/config/riscv/riscv-vsetvl.h | 1 +
>> 2 files changed, 31 insertions(+), 16 deletions(-)
>>
>> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
>> index f7ae6c16bee..73d672b083b 100644
>> --- a/gcc/config/riscv/riscv-vsetvl.cc
>> +++ b/gcc/config/riscv/riscv-vsetvl.cc
>> @@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info,
>> return new_info;
>> }
>>
>> +/* Wrapper helps to return the AVL or VL operand for the
>> + vector_insn_info. Return AVL if the AVL is not VLMAX.
>> + Otherwise, return the VL operand. */
>> +rtx
>> +vector_insn_info::get_avl_or_vl_reg (void) const
>> +{
>> + gcc_assert (has_avl_reg ());
>> + if (!vlmax_avl_p (get_avl ()))
>> + return get_avl ();
>> +
>> + if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ()))
>> + return ::get_vl (get_insn ()->rtl ());
>> +
>> + if (get_avl_source ())
>> + return get_avl_reg_rtx ();
>> +
>> + /* A DIRTY (polluted EMPTY) block if:
>> + - get_insn is scalar move (no AVL or VL operand).
>> + - get_avl_source is null (no def in the current DIRTY block).
>> + Then we trace the previous insn which must be the insn
>> + already inserted in Phase 2 to get the VL operand for VLMAX. */
>> + rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ());
>> + gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn));
>> + return ::get_vl (prev_rinsn);
>> +}
>> +
>> bool
>> vector_insn_info::update_fault_first_load_avl (insn_info *insn)
>> {
>> @@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void)
>> bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i);
>> else if (expr->has_avl_reg ())
>> {
>> - rtx avl = vlmax_avl_p (expr->get_avl ())
>> - ? get_vl (expr->get_insn ()->rtl ())
>> - : expr->get_avl ();
>> + rtx reg = expr->get_avl_or_vl_reg ();
>> for (const insn_info *insn : bb->real_nondebug_insns ())
>> {
>> - if (find_access (insn->defs (), REGNO (avl)))
>> + if (find_access (insn->defs (), REGNO (reg)))
>> {
>> bitmap_clear_bit (
>> m_vector_manager->vector_transp[curr_bb_idx], i);
>> break;
>> }
>> else if (vlmax_avl_p (expr->get_avl ())
>> - && find_access (insn->uses (), REGNO (avl)))
>> + && find_access (insn->uses (), REGNO (reg)))
>> {
>> bitmap_clear_bit (
>> m_vector_manager->vector_transp[curr_bb_idx], i);
>> @@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void)
>> = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX);
>> else if (vlmax_avl_p (reaching_out.get_avl ()))
>> {
>> - rtx vl = NULL_RTX;
>> - /* For user VSETVL VL, AVL. We need to use VL operand here, so we
>> - don't directly use get_avl_reg_rtx (). Instead, we use the VL
>> - of the INSN->RTL (). */
>> - if (!reaching_out.get_avl_source ())
>> - {
>> - gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ()));
>> - vl = get_vl (reaching_out.get_insn ()->rtl ());
>> - }
>> - else
>> - vl = reaching_out.get_avl_reg_rtx ();
>> + rtx vl = reaching_out.get_avl_or_vl_reg ();
>> new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl);
>> }
>> else
>> diff --git a/gcc/config/riscv/riscv-vsetvl.h b/gcc/config/riscv/riscv-vsetvl.h
>> index 4b5825d7f6b..2a315e45f31 100644
>> --- a/gcc/config/riscv/riscv-vsetvl.h
>> +++ b/gcc/config/riscv/riscv-vsetvl.h
>> @@ -335,6 +335,7 @@ public:
>>
>> rtl_ssa::insn_info *get_insn () const { return m_insn; }
>> const bool *get_demands (void) const { return m_demands; }
>> + rtx get_avl_or_vl_reg (void) const;
>> rtx get_avl_reg_rtx (void) const
>> {
>> return gen_rtx_REG (Pmode, get_avl_source ()->regno ());
>> --
>> 2.36.3
>>
@@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info,
return new_info;
}
+/* Wrapper helps to return the AVL or VL operand for the
+ vector_insn_info. Return AVL if the AVL is not VLMAX.
+ Otherwise, return the VL operand. */
+rtx
+vector_insn_info::get_avl_or_vl_reg (void) const
+{
+ gcc_assert (has_avl_reg ());
+ if (!vlmax_avl_p (get_avl ()))
+ return get_avl ();
+
+ if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ()))
+ return ::get_vl (get_insn ()->rtl ());
+
+ if (get_avl_source ())
+ return get_avl_reg_rtx ();
+
+ /* A DIRTY (polluted EMPTY) block if:
+ - get_insn is scalar move (no AVL or VL operand).
+ - get_avl_source is null (no def in the current DIRTY block).
+ Then we trace the previous insn which must be the insn
+ already inserted in Phase 2 to get the VL operand for VLMAX. */
+ rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ());
+ gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn));
+ return ::get_vl (prev_rinsn);
+}
+
bool
vector_insn_info::update_fault_first_load_avl (insn_info *insn)
{
@@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void)
bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i);
else if (expr->has_avl_reg ())
{
- rtx avl = vlmax_avl_p (expr->get_avl ())
- ? get_vl (expr->get_insn ()->rtl ())
- : expr->get_avl ();
+ rtx reg = expr->get_avl_or_vl_reg ();
for (const insn_info *insn : bb->real_nondebug_insns ())
{
- if (find_access (insn->defs (), REGNO (avl)))
+ if (find_access (insn->defs (), REGNO (reg)))
{
bitmap_clear_bit (
m_vector_manager->vector_transp[curr_bb_idx], i);
break;
}
else if (vlmax_avl_p (expr->get_avl ())
- && find_access (insn->uses (), REGNO (avl)))
+ && find_access (insn->uses (), REGNO (reg)))
{
bitmap_clear_bit (
m_vector_manager->vector_transp[curr_bb_idx], i);
@@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void)
= gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX);
else if (vlmax_avl_p (reaching_out.get_avl ()))
{
- rtx vl = NULL_RTX;
- /* For user VSETVL VL, AVL. We need to use VL operand here, so we
- don't directly use get_avl_reg_rtx (). Instead, we use the VL
- of the INSN->RTL (). */
- if (!reaching_out.get_avl_source ())
- {
- gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ()));
- vl = get_vl (reaching_out.get_insn ()->rtl ());
- }
- else
- vl = reaching_out.get_avl_reg_rtx ();
+ rtx vl = reaching_out.get_avl_or_vl_reg ();
new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl);
}
else
@@ -335,6 +335,7 @@ public:
rtl_ssa::insn_info *get_insn () const { return m_insn; }
const bool *get_demands (void) const { return m_demands; }
+ rtx get_avl_or_vl_reg (void) const;
rtx get_avl_reg_rtx (void) const
{
return gen_rtx_REG (Pmode, get_avl_source ()->regno ());