Ajit Agarwal <[email protected]> writes:
> +static void
> +update_change (set_info *set)
> +{
> + if (!set->has_any_uses ())
> + return;
> +
> + auto *use = *set->all_uses ().begin ();
> + do
> + {
> + auto *next_use = use->next_use ();
> + if (use->is_in_debug_insn ())
> + substitute_debug_use (use);
> + else if (use->is_in_phi ())
> + {
> + update_change (use->phi ());
> + }
> + else
> + {
> + crtl->ssa->remove_use (use);
> + }
> + use = next_use;
> + }
> + while (use);
> +}
This still contains direct modifications to the rtl-ssa ir (remove_use).
Which case is it handling? REG_EQUAL notes?
The patch shouldn't rely on:
> diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h
> index 567701c7995..3003251e62c 100644
> --- a/gcc/rtl-ssa/functions.h
> +++ b/gcc/rtl-ssa/functions.h
> @@ -222,6 +222,13 @@ public:
> template<typename T, typename... Ts>
> T *change_alloc (obstack_watermark &wm, Ts... args);
>
> + auto_vec<access_info *> &get_m_temp_defs () { return m_temp_defs; }
> +
> + template<typename T, typename... Ts>
> + T *allocate (Ts... args);
> +
> + void remove_use (use_info *);
> +
> private:
> class bb_phi_info;
> class build_info;
Those aren't things that rs6000 code should be doing directly.
For:
> +// Set subreg with use of INSN given SRC rtx instruction.
> +static void
> +set_load_subreg (insn_info *i1, rtx src)
> +{
> + rtx set = single_set (i1->rtl());
> + rtx old_dest = SET_DEST (set);
> +
> + for (auto def : i1->defs ())
> + {
> + auto set = dyn_cast<set_info *> (def);
> + for (auto use : set->nondebug_insn_uses ())
> + {
> + insn_info *info = use->insn ();
> + if (!info || !info->rtl ())
> + continue;
I think this should check use->is_artificial () instead. If that's false,
then use->insn () and use->insn ()->rtl () should both be nonnull, and there
should be no need to check.
> +
> + rtx_insn *rtl_insn = info->rtl ();
> + insn_propagation prop (rtl_insn, old_dest, src);
> + if (!prop.apply_to_pattern (&PATTERN (rtl_insn)))
> + gcc_assert (0);
> + }
> + }
> +}
Could you combine this with the code that creates the insn_change
for the insn, rather than doing that in a separate function?
IMO it's better to keep the creation of insn_changes together
with the changes that they describe.
Thanks,
Richard
