public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
@ 2023-11-06 13:01 Maxim Blinov
  2023-11-06 13:04 ` Richard Biener
  2023-11-09 16:26 ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Maxim Blinov @ 2023-11-06 13:01 UTC (permalink / raw)
  To: gcc-patches; +Cc: juzhe.zhong, maxim.blinov, maxim.a.blinov

From: Maxim Blinov <maxim.blinov@imgtec.com>

This patch is based on and intended for the vendors/riscv/gcc-13-with-riscv-opts branch - please apply if looks OK.

Fixes the following ICEs that I'm seeing:

FAIL: gcc.dg/vect/O3-pr49087.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/no-scevccp-pr86725-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/no-scevccp-pr86725-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/no-scevccp-pr86725-3.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/no-scevccp-pr86725-4.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/pr94443.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/pr94443.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/slp-50.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/slp-50.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/vect-cond-13.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/vect-cond-13.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/vect-live-6.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.dg/vect/vect-live-6.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.target/riscv/rvv/autovec/partial/live-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
FAIL: gcc.target/riscv/rvv/autovec/partial/live-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)

-- >8 --

When we create a VEC_EXPAND gimple stmt:

          /* SCALAR_RES = VEC_EXTRACT <VEC_LHS, LEN + BIAS - 1>.  */
          tree scalar_res
            = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
                            vec_lhs_phi, last_index);

Under the hood we are really just creating a GIMPLE_CALL stmt. Later
on, when we `gsi_insert_seq_before` our stmts:

      if (stmts)
        {
          gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
          gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);

We eventually run into tree-ssa-operands.cc:1147:

  operands_scanner (fn, stmt).build_ssa_operands ();

Since VEC_EXPAND is *not* marked with ECF_NOVOPS, ECF_CONST, or
ECF_PURE flags in internal-fn.def, when
`operand_scanner::parse_ssa_operands` comes across our
VEC_EXTRACT-type GIMPLE_CALL, it generates a `gimple_vop()` artificial
variable.

`operand_scanner::finalize_ssa_defs` then picks this up, so our final
stmt goes from

_73 = .VEC_EXTRACT (vect_last_9.56_71, _72);

to

# .MEM = VDEF <>
_73 = .VEC_EXTRACT (vect_last_9.56_71, _72);

But more importantly it marks us as `ssa_renaming_needed`, in
tree-ssa-operands.cc:420:

  /* If we have a non-SSA_NAME VDEF, mark it for renaming.  */
  if (gimple_vdef (stmt)
      && TREE_CODE (gimple_vdef (stmt)) != SSA_NAME)
    {
      fn->gimple_df->rename_vops = 1;
      fn->gimple_df->ssa_renaming_needed = 1;
    }

This then proceeds to crash the compiler when we are about to leave
`vect_transform_loops`:

  if (need_ssa_update_p (cfun))
    {
      gcc_assert (loop_vinfo->any_known_not_updated_vssa);
      fun->gimple_df->ssa_renaming_needed = false;
      todo |= TODO_update_ssa_only_virtuals;
    }

Since,

- `need_ssa_update_p (cfun)` is true (it was set when we generated a
  memory vdef)
- `loop_vinfo->any_known_not_updated_vssa` is false

As the code currently stands, creating a gimple stmt containing a
VEC_EXTRACT should always generate a memory vdef, therefore we should
remember to mark `loop_vinfo->any_known_not_updated_vssa` afterwards.

gcc/ChangeLog:

	* tree-vect-loop.cc (vectorizable_live_operation): Remember to
	assert loop_vinfo->any_known_not_updated_vssa if we are inserting
	a call to VEC_EXPAND.
---
 gcc/tree-vect-loop.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index c8df2c88575..53c3a31d2a8 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -10155,6 +10155,11 @@ vectorizable_live_operation (vec_info *vinfo,
 	    = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
 			    vec_lhs_phi, last_index);
 
+	  /* We've expanded SSA at this point, and since VEC_EXTRACT
+	     will generate vops, make sure to tell GCC that we need to
+	     update SSA.  */
+	  loop_vinfo->any_known_not_updated_vssa = true;
+
 	  /* Convert the extracted vector element to the scalar type.  */
 	  new_tree = gimple_convert (&stmts, lhs_type, scalar_res);
 	}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:01 [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa Maxim Blinov
@ 2023-11-06 13:04 ` Richard Biener
  2023-11-06 13:12   ` Maxim Blinov
  2023-11-09 16:26 ` Jeff Law
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Biener @ 2023-11-06 13:04 UTC (permalink / raw)
  To: Maxim Blinov; +Cc: gcc-patches, juzhe.zhong, maxim.blinov

On Mon, Nov 6, 2023 at 2:02 PM Maxim Blinov <maxim.a.blinov@gmail.com> wrote:
>
> From: Maxim Blinov <maxim.blinov@imgtec.com>
>
> This patch is based on and intended for the vendors/riscv/gcc-13-with-riscv-opts branch - please apply if looks OK.
>
> Fixes the following ICEs that I'm seeing:
>
> FAIL: gcc.dg/vect/O3-pr49087.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-3.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-4.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/pr94443.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/pr94443.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/slp-50.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/slp-50.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-cond-13.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-cond-13.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-live-6.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-live-6.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.target/riscv/rvv/autovec/partial/live-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.target/riscv/rvv/autovec/partial/live-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
>
> -- >8 --
>
> When we create a VEC_EXPAND gimple stmt:
>
>           /* SCALAR_RES = VEC_EXTRACT <VEC_LHS, LEN + BIAS - 1>.  */
>           tree scalar_res
>             = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
>                             vec_lhs_phi, last_index);
>
> Under the hood we are really just creating a GIMPLE_CALL stmt. Later
> on, when we `gsi_insert_seq_before` our stmts:
>
>       if (stmts)
>         {
>           gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
>           gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
>
> We eventually run into tree-ssa-operands.cc:1147:
>
>   operands_scanner (fn, stmt).build_ssa_operands ();
>
> Since VEC_EXPAND is *not* marked with ECF_NOVOPS, ECF_CONST, or
> ECF_PURE flags in internal-fn.def, when

I see

DEF_INTERNAL_OPTAB_FN (VEC_EXTRACT, ECF_CONST | ECF_NOTHROW,
                       vec_extract, vec_extract)

?

> `operand_scanner::parse_ssa_operands` comes across our
> VEC_EXTRACT-type GIMPLE_CALL, it generates a `gimple_vop()` artificial
> variable.
>
> `operand_scanner::finalize_ssa_defs` then picks this up, so our final
> stmt goes from
>
> _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
>
> to
>
> # .MEM = VDEF <>
> _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
>
> But more importantly it marks us as `ssa_renaming_needed`, in
> tree-ssa-operands.cc:420:
>
>   /* If we have a non-SSA_NAME VDEF, mark it for renaming.  */
>   if (gimple_vdef (stmt)
>       && TREE_CODE (gimple_vdef (stmt)) != SSA_NAME)
>     {
>       fn->gimple_df->rename_vops = 1;
>       fn->gimple_df->ssa_renaming_needed = 1;
>     }
>
> This then proceeds to crash the compiler when we are about to leave
> `vect_transform_loops`:
>
>   if (need_ssa_update_p (cfun))
>     {
>       gcc_assert (loop_vinfo->any_known_not_updated_vssa);
>       fun->gimple_df->ssa_renaming_needed = false;
>       todo |= TODO_update_ssa_only_virtuals;
>     }
>
> Since,
>
> - `need_ssa_update_p (cfun)` is true (it was set when we generated a
>   memory vdef)
> - `loop_vinfo->any_known_not_updated_vssa` is false
>
> As the code currently stands, creating a gimple stmt containing a
> VEC_EXTRACT should always generate a memory vdef, therefore we should
> remember to mark `loop_vinfo->any_known_not_updated_vssa` afterwards.
>
> gcc/ChangeLog:
>
>         * tree-vect-loop.cc (vectorizable_live_operation): Remember to
>         assert loop_vinfo->any_known_not_updated_vssa if we are inserting
>         a call to VEC_EXPAND.
> ---
>  gcc/tree-vect-loop.cc | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index c8df2c88575..53c3a31d2a8 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -10155,6 +10155,11 @@ vectorizable_live_operation (vec_info *vinfo,
>             = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
>                             vec_lhs_phi, last_index);
>
> +         /* We've expanded SSA at this point, and since VEC_EXTRACT
> +            will generate vops, make sure to tell GCC that we need to
> +            update SSA.  */
> +         loop_vinfo->any_known_not_updated_vssa = true;
> +
>           /* Convert the extracted vector element to the scalar type.  */
>           new_tree = gimple_convert (&stmts, lhs_type, scalar_res);
>         }
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:04 ` Richard Biener
@ 2023-11-06 13:12   ` Maxim Blinov
  2023-11-06 13:18     ` Kito Cheng
  0 siblings, 1 reply; 8+ messages in thread
From: Maxim Blinov @ 2023-11-06 13:12 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, juzhe.zhong, maxim.blinov

On Mon, 6 Nov 2023 at 13:07, Richard Biener <richard.guenther@gmail.com> wrote:
> I see
>
> DEF_INTERNAL_OPTAB_FN (VEC_EXTRACT, ECF_CONST | ECF_NOTHROW,
>                        vec_extract, vec_extract)
>
> ?

Oh, you're right! I should have checked the master branch first... and
I was even wondering why it wasn't marked as such. Should perhaps
cherry pick this for gcc-13-with-riscv-opts?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:12   ` Maxim Blinov
@ 2023-11-06 13:18     ` Kito Cheng
  2023-11-06 14:39       ` Jeff Law
  2023-11-07  3:55       ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Kito Cheng @ 2023-11-06 13:18 UTC (permalink / raw)
  To: Maxim Blinov, Jeff Law
  Cc: Richard Biener, gcc-patches, juzhe.zhong, maxim.blinov

> Oh, you're right! I should have checked the master branch first... and
> I was even wondering why it wasn't marked as such. Should perhaps
> cherry pick this for gcc-13-with-riscv-opts?

 gcc-13-with-riscv-opts mostly maintained by Ventana folks, so maybe
ask Jeff if you want to cherry pick into that branch?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:18     ` Kito Cheng
@ 2023-11-06 14:39       ` Jeff Law
  2023-11-07  3:55       ` Jeff Law
  1 sibling, 0 replies; 8+ messages in thread
From: Jeff Law @ 2023-11-06 14:39 UTC (permalink / raw)
  To: Kito Cheng, Maxim Blinov
  Cc: Richard Biener, gcc-patches, juzhe.zhong, maxim.blinov



On 11/6/23 06:18, Kito Cheng wrote:
>> Oh, you're right! I should have checked the master branch first... and
>> I was even wondering why it wasn't marked as such. Should perhaps
>> cherry pick this for gcc-13-with-riscv-opts?
> 
>   gcc-13-with-riscv-opts mostly maintained by Ventana folks, so maybe
> ask Jeff if you want to cherry pick into that branch?
I cherry pick all all the riscv bits over to that branch, usually once a 
week.

Jeff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:18     ` Kito Cheng
  2023-11-06 14:39       ` Jeff Law
@ 2023-11-07  3:55       ` Jeff Law
  1 sibling, 0 replies; 8+ messages in thread
From: Jeff Law @ 2023-11-07  3:55 UTC (permalink / raw)
  To: Kito Cheng, Maxim Blinov, Jeff Law
  Cc: Richard Biener, gcc-patches, juzhe.zhong, maxim.blinov



On 11/6/23 06:18, Kito Cheng wrote:
>> Oh, you're right! I should have checked the master branch first... and
>> I was even wondering why it wasn't marked as such. Should perhaps
>> cherry pick this for gcc-13-with-riscv-opts?
> 
>   gcc-13-with-riscv-opts mostly maintained by Ventana folks, so maybe
> ask Jeff if you want to cherry pick into that branch?
I haven't done the risc-v bits this week (yet, probably a job for the 
short flight tomorrow).  But I did go ahead and cherry-pick the 
appropriate generic bits to make the relevant IFNs const/nothrow.

jeff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-06 13:01 [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa Maxim Blinov
  2023-11-06 13:04 ` Richard Biener
@ 2023-11-09 16:26 ` Jeff Law
  2023-11-09 22:05   ` Maxim Blinov
  1 sibling, 1 reply; 8+ messages in thread
From: Jeff Law @ 2023-11-09 16:26 UTC (permalink / raw)
  To: Maxim Blinov, gcc-patches; +Cc: juzhe.zhong, maxim.blinov



On 11/6/23 06:01, Maxim Blinov wrote:
> From: Maxim Blinov <maxim.blinov@imgtec.com>
> 
> This patch is based on and intended for the vendors/riscv/gcc-13-with-riscv-opts branch - please apply if looks OK.
> 
> Fixes the following ICEs that I'm seeing:
> 
> FAIL: gcc.dg/vect/O3-pr49087.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-3.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/no-scevccp-pr86725-4.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/pr94443.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/pr94443.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/slp-50.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/slp-50.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-cond-13.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-cond-13.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-live-6.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.dg/vect/vect-live-6.c -flto -ffat-lto-objects (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.target/riscv/rvv/autovec/partial/live-1.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> FAIL: gcc.target/riscv/rvv/autovec/partial/live-2.c (internal compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> 
> -- >8 --
> 
> When we create a VEC_EXPAND gimple stmt:
> 
>            /* SCALAR_RES = VEC_EXTRACT <VEC_LHS, LEN + BIAS - 1>.  */
>            tree scalar_res
>              = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
>                              vec_lhs_phi, last_index);
> 
> Under the hood we are really just creating a GIMPLE_CALL stmt. Later
> on, when we `gsi_insert_seq_before` our stmts:
> 
>        if (stmts)
>          {
>            gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
>            gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
> 
> We eventually run into tree-ssa-operands.cc:1147:
> 
>    operands_scanner (fn, stmt).build_ssa_operands ();
> 
> Since VEC_EXPAND is *not* marked with ECF_NOVOPS, ECF_CONST, or
> ECF_PURE flags in internal-fn.def, when
> `operand_scanner::parse_ssa_operands` comes across our
> VEC_EXTRACT-type GIMPLE_CALL, it generates a `gimple_vop()` artificial
> variable.
> 
> `operand_scanner::finalize_ssa_defs` then picks this up, so our final
> stmt goes from
> 
> _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
> 
> to
> 
> # .MEM = VDEF <>
> _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
> 
> But more importantly it marks us as `ssa_renaming_needed`, in
> tree-ssa-operands.cc:420:
> 
>    /* If we have a non-SSA_NAME VDEF, mark it for renaming.  */
>    if (gimple_vdef (stmt)
>        && TREE_CODE (gimple_vdef (stmt)) != SSA_NAME)
>      {
>        fn->gimple_df->rename_vops = 1;
>        fn->gimple_df->ssa_renaming_needed = 1;
>      }
> 
> This then proceeds to crash the compiler when we are about to leave
> `vect_transform_loops`:
> 
>    if (need_ssa_update_p (cfun))
>      {
>        gcc_assert (loop_vinfo->any_known_not_updated_vssa);
>        fun->gimple_df->ssa_renaming_needed = false;
>        todo |= TODO_update_ssa_only_virtuals;
>      }
> 
> Since,
> 
> - `need_ssa_update_p (cfun)` is true (it was set when we generated a
>    memory vdef)
> - `loop_vinfo->any_known_not_updated_vssa` is false
> 
> As the code currently stands, creating a gimple stmt containing a
> VEC_EXTRACT should always generate a memory vdef, therefore we should
> remember to mark `loop_vinfo->any_known_not_updated_vssa` afterwards.
> 
> gcc/ChangeLog:
> 
> 	* tree-vect-loop.cc (vectorizable_live_operation): Remember to
> 	assert loop_vinfo->any_known_not_updated_vssa if we are inserting
> 	a call to VEC_EXPAND.
Just to avoid any doubt -- with the internal-fn.def patch I cherry 
picked earlier this week to the branch, this is no longer needed, right?

jeff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
  2023-11-09 16:26 ` Jeff Law
@ 2023-11-09 22:05   ` Maxim Blinov
  0 siblings, 0 replies; 8+ messages in thread
From: Maxim Blinov @ 2023-11-09 22:05 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, juzhe.zhong, maxim.blinov

[-- Attachment #1: Type: text/plain, Size: 5028 bytes --]

Yes, those tests that triggered the ICE now pass.

Maxim


On Thu, 9 Nov 2023 at 16:26, Jeff Law <jeffreyalaw@gmail.com> wrote:

>
>
> On 11/6/23 06:01, Maxim Blinov wrote:
> > From: Maxim Blinov <maxim.blinov@imgtec.com>
> >
> > This patch is based on and intended for the
> vendors/riscv/gcc-13-with-riscv-opts branch - please apply if looks OK.
> >
> > Fixes the following ICEs that I'm seeing:
> >
> > FAIL: gcc.dg/vect/O3-pr49087.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/no-scevccp-pr86725-1.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/no-scevccp-pr86725-2.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/no-scevccp-pr86725-3.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/no-scevccp-pr86725-4.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/pr94443.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/pr94443.c -flto -ffat-lto-objects (internal compiler
> error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/slp-50.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/slp-50.c -flto -ffat-lto-objects (internal compiler
> error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/vect-cond-13.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/vect-cond-13.c -flto -ffat-lto-objects (internal
> compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/vect-live-6.c (internal compiler error: in
> vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.dg/vect/vect-live-6.c -flto -ffat-lto-objects (internal
> compiler error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.target/riscv/rvv/autovec/partial/live-1.c (internal compiler
> error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> > FAIL: gcc.target/riscv/rvv/autovec/partial/live-2.c (internal compiler
> error: in vect_transform_loops, at tree-vectorizer.cc:1032)
> >
> > -- >8 --
> >
> > When we create a VEC_EXPAND gimple stmt:
> >
> >            /* SCALAR_RES = VEC_EXTRACT <VEC_LHS, LEN + BIAS - 1>.  */
> >            tree scalar_res
> >              = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE
> (vectype),
> >                              vec_lhs_phi, last_index);
> >
> > Under the hood we are really just creating a GIMPLE_CALL stmt. Later
> > on, when we `gsi_insert_seq_before` our stmts:
> >
> >        if (stmts)
> >          {
> >            gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
> >            gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
> >
> > We eventually run into tree-ssa-operands.cc:1147:
> >
> >    operands_scanner (fn, stmt).build_ssa_operands ();
> >
> > Since VEC_EXPAND is *not* marked with ECF_NOVOPS, ECF_CONST, or
> > ECF_PURE flags in internal-fn.def, when
> > `operand_scanner::parse_ssa_operands` comes across our
> > VEC_EXTRACT-type GIMPLE_CALL, it generates a `gimple_vop()` artificial
> > variable.
> >
> > `operand_scanner::finalize_ssa_defs` then picks this up, so our final
> > stmt goes from
> >
> > _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
> >
> > to
> >
> > # .MEM = VDEF <>
> > _73 = .VEC_EXTRACT (vect_last_9.56_71, _72);
> >
> > But more importantly it marks us as `ssa_renaming_needed`, in
> > tree-ssa-operands.cc:420:
> >
> >    /* If we have a non-SSA_NAME VDEF, mark it for renaming.  */
> >    if (gimple_vdef (stmt)
> >        && TREE_CODE (gimple_vdef (stmt)) != SSA_NAME)
> >      {
> >        fn->gimple_df->rename_vops = 1;
> >        fn->gimple_df->ssa_renaming_needed = 1;
> >      }
> >
> > This then proceeds to crash the compiler when we are about to leave
> > `vect_transform_loops`:
> >
> >    if (need_ssa_update_p (cfun))
> >      {
> >        gcc_assert (loop_vinfo->any_known_not_updated_vssa);
> >        fun->gimple_df->ssa_renaming_needed = false;
> >        todo |= TODO_update_ssa_only_virtuals;
> >      }
> >
> > Since,
> >
> > - `need_ssa_update_p (cfun)` is true (it was set when we generated a
> >    memory vdef)
> > - `loop_vinfo->any_known_not_updated_vssa` is false
> >
> > As the code currently stands, creating a gimple stmt containing a
> > VEC_EXTRACT should always generate a memory vdef, therefore we should
> > remember to mark `loop_vinfo->any_known_not_updated_vssa` afterwards.
> >
> > gcc/ChangeLog:
> >
> >       * tree-vect-loop.cc (vectorizable_live_operation): Remember to
> >       assert loop_vinfo->any_known_not_updated_vssa if we are inserting
> >       a call to VEC_EXPAND.
> Just to avoid any doubt -- with the internal-fn.def patch I cherry
> picked earlier this week to the branch, this is no longer needed, right?
>
> jeff
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-11-09 22:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 13:01 [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa Maxim Blinov
2023-11-06 13:04 ` Richard Biener
2023-11-06 13:12   ` Maxim Blinov
2023-11-06 13:18     ` Kito Cheng
2023-11-06 14:39       ` Jeff Law
2023-11-07  3:55       ` Jeff Law
2023-11-09 16:26 ` Jeff Law
2023-11-09 22:05   ` Maxim Blinov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).