Re: [PATCH] RISC-V: VECT: Remember to assert any_known_not_updated_vssa

[Maxim Blinov <maxim.a.blinov@gmail.com>]

[-- 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
>