https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123833
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Roger Sayle from comment #4)
> Here's my proposed fix:
>
> diff --git a/gcc/fwprop.cc b/gcc/fwprop.cc
> index 68969d10401..46ad11db5cf 100644
> --- a/gcc/fwprop.cc
> +++ b/gcc/fwprop.cc
> @@ -925,6 +925,10 @@ fwprop_done (void)
> fprintf (dump_file,
> "\nNumber of successful forward propagations: %d\n\n",
> num_changes);
> +
> + /* Clear insn_extract cache to avoid stale insn attributes. */
> + if (num_changes)
> + recog_data.insn = nullptr;
> }
>
> /* Try to optimize INSN, returning true if something changes.
>
>
>
> The MIPS backend is more fragile than most targets to the correctness of
> instruction attibutes, which are used for instruction costing. Initially,
> insn #27 is
>
> (insn 27 26 28 4 (set (reg:DI 207)
> (zero_extend:DI (reg:SI 206 [ _10 ]))) "../../pr123833.c":18:13
> discrim
> 1 223 {*zero_extendsidi2}
> (nil))
>
> which gets recognized/cached by insn_extract. During fwprop this
> instruction is converted to:
>
> insn 27 24 28 4 (set (reg:DI 207 [ _10 ])
> (and:DI (reg:DI 198 [ _21 ])
> (const_int 255 [0xff]))) "../../pr123833.c":18:13 discrim 1 192
> {*an
> ddi3}
> (nil))
>
> but recog's extract_insn cache isn't cleared. So when in the following
> pass, ce1, the code calls mips_insn_cost, it calls get_attr_insn_count which
> finds a cached entry with matching insn pointer, but with stale data,
> specifically which_alternative, is for a *zero_extendsidi2 not for a
> *anddi3. Mistakenly thinking the instruction is now of type "load" it hits
> the assertion that the operand must be a MEM.
>
> Finally, the reason that this is placed in fwprop_done rather than the
> perhaps more obvious end of "try_fwprop_subst_pattern" in the chunk:
>
> confirm_change_group ();
> crtl->ssa->change_insn (use_change);
> + recog_data.insn = nullptr;
> num_changes++;
> return true;
>
> (which was my first attempt) is that this something/somewhere undoes this
> change, so that recog_data.insn is "conveniently" returned to its stale
> value by the end of the pass.
>
> Any thoughts on a better fix? (whilst I bootstrap and regression test on
> x86_64).
Maybe it should go into `rtl_ssa::function_info::change_insns` instead.
I suspect late_combine will also cause a similar issue after this call:
crtl->ssa->change_insns (m_nondebug_changes);
Or maybe inside rtl_ssa::function_info::~function_info is the better place.
rtl_ssa::function_info gets deleted at the end of the pass in most (if not all
cases).
