Hello,
On Tue, 8 Sep 2020, Richard Biener wrote:
> CCing some people to double-check my graph partitioning algorithm.
I seem to not know the pre-existing data structures enough to say anything
about this, but I noticed small things which might or might not indicate
thinkos or incompleteness:
> +static void
> +vect_bb_partition_graph_r (bb_vec_info bb_vinfo,
> + slp_instance instance, slp_tree node,
> + hash_map<stmt_vec_info, slp_instance>
> &stmt_to_instance,
> + hash_map<slp_instance, slp_instance>
> &instance_leader)
> +{
> + stmt_vec_info stmt_info;
> + unsigned i;
> + bool all = true;
> + FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt_info)
> + {
> + bool existed_p;
> + slp_instance &stmt_instance
> + = stmt_to_instance.get_or_insert (stmt_info, &existed_p);
> + if (!existed_p)
> + {
> + all = false;
> + }
> + else if (stmt_instance != instance)
> + {
> + /* If we're running into a previously marked stmt make us the
> + leader of the current ultimate leader. This keeps the
> + leader chain acyclic and works even when the current instance
> + connects two previously independent graph parts. */
> + stmt_instance = get_ultimate_leader (stmt_instance, instance_leader);
> + if (stmt_instance != instance)
> + instance_leader.put (stmt_instance, instance);
> + }
> + stmt_instance = instance;
This last assignment is useless.
> +/* Partition the SLP graph into pieces that can be costed independently. */
> +
> +static void
> +vect_bb_partition_graph (bb_vec_info bb_vinfo)
> +{
...
> + /* Then collect entries to each independent subgraphs. */
> + for (unsigned i = 0; bb_vinfo->slp_instances.iterate (i, &instance); ++i)
> + {
> + slp_instance leader = get_ultimate_leader (instance, instance_leader);
> + if (leader == instance)
> + leader->subgraph_entries.safe_push (leader);
> + else
> + {
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE, vect_location,
> + "instance %p is leader of %p\n",
> + leader, instance);
> + leader->subgraph_entries.safe_push (instance);
> + }
So the 'leader->subgraph_entries.safe_push (instance)' is actually done
unconditionally (the leader is leader of itself), only the debug dump is
conditional.
Ciao,
Michael.
