Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
Yes, those tests that triggered the ICE now pass.
Maxim
On Thu, 9 Nov 2023 at 16:26, Jeff Law wrote:
>
>
> On 11/6/23 06:01, Maxim Blinov wrote:
> > From: Maxim Blinov
> >
> > 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 . */
> >tree scalar_res
> > = gimple_build (, 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 (_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
>
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
On 11/6/23 06:01, Maxim Blinov wrote:
From: Maxim Blinov
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 . */
tree scalar_res
= gimple_build (, 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 (_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
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
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
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
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
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
Not sure who is maintaining this branch. I always developing on the master. CCing to other riscv folks Replied Message FromMaxim BlinovDate11/06/2023 21:13 ToRichard Biener Ccgcc-patches@gcc.gnu.org,juzhe.zh...@rivai.ai,maxim.bli...@imgtec.comSubjectRe: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssaOn Mon, 6 Nov 2023 at 13:07, Richard Biener 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?
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
> 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?
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
On Mon, 6 Nov 2023 at 13:07, Richard Biener 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?
Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa
On Mon, Nov 6, 2023 at 2:02 PM Maxim Blinov wrote:
>
> From: Maxim Blinov
>
> 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 . */
> tree scalar_res
> = gimple_build (, 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 (_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 (, CFN_VEC_EXTRACT, TREE_TYPE
