This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [graphite] SCoP detection update to fix Bug 37372 and various others
- From: "Sebastian Pop" <sebpop at gmail dot com>
- To: "Tobias Grosser" <grosser at fim dot uni-passau dot de>
- Cc: "GCC Patches" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 26 Sep 2008 12:42:47 -0500
- Subject: Re: [graphite] SCoP detection update to fix Bug 37372 and various others
- References: <1222437377.1486.77.camel@localhost>
> +/* A single entry single exit region, defined using bbs as borders.
> + All controlflow touching this region, comes in passing ENTRY and
s/controlflow/control flow/
> + leaves passing EXIT. By using bbs instead of edges for the borders
> + we are able to represent also SESE regions, that do not have a single
> + entry or exit edge.
If they do not have a single entry or single exit, then these regions
are not SESE regions:
s/represent also SESE regions, that/represent also regions that/
> +typedef struct sd_region_p
> +{
> + /* The entry bb dominates all bbs in the SESE region. It is part of the
s/in the SESE region/in the region/
> + region. */
> + basic_block entry;
> +
> + /* The exit bb postdominates all bbs in the SESE region, BUT is NOT
s/in the SESE region/in the region/
Do not use capital letters for BUT and NOT.
> + part of the region. */
> + basic_block exit;
> +} sd_region;
> /* Checks, if a bb can be added to a SCoP. */
>
> static struct scopdet_info
> -scopdet_edge_info (edge ee,
> - VEC (scop_p, heap) **scops, gbb_type type, gimple *stmt)
> -
> +scopdet_edge_info (basic_block bb, VEC (sd_region, heap) **scops, gbb_type type)
You should also rename this function into scopdet_bb_info now, and
please document the parameters.
> + /* XXX: ENTRY_BLOCK_PTR could be optimized in later steps. */
> + stmt = harmful_stmt_in_bb (ENTRY_BLOCK_PTR, bb);
We could use, instead of ENTRY_BLOCK_PTR, the exit of the last closed
scop that has been copied in the scops vector, i.e. when
result.difficult was set. You could store a pointer to that bb in a
static variable when you call move_sd_regions.
> + return dominated_by_p (CDI_DOMINATORS, bb, region->entry)
one space between return and dominated_by_p.
> + gcc_assert(find_single_entry_edge (region));
one space between gcc_assert and open parenthesis.
> + sd_region *r = (sd_region*) e->aux;
one space between sd_region and *.
> + /* We create a forwarder bb (5) for all edges leaving this region
> + (3->5, 4->5). All other edges leading to the same bb, are moved
> + to a new bb (6). If these edges where part of another region (2->5)
> + we update the region->exit pointer, of this region.
point space space new sentence.
s/where/were/
> + Now there is only a single exit edge (5->6). */
point space space close comment.
The patch looks good, but somebody has to approve it.
Thanks,
Sebastian