Re: [Graphite] Add analysis pass to build the refined program structure tree.

[Sebastian Pop <sebpop at gmail dot com>]

On Thu, May 27, 2010 at 01:43, Tobias Grosser <grosser@fim.uni-passau.de> wrote:
> Here an updated version.

+#ifdef HAVE_cloog
+#include "refined-regions.h"

Why do we have all this code conditionally compiled on HAVE_cloog?

+/* Check is BB is element of REGION.  */

This should be /* Check that BB is an element of REGION.  */

+/* Check is the INNER regions is contained in the OUTER region.  */
+/* Check if BB is part of the dominance frontier of ENTRY, because it was
+/* Check if ENTRY and EXIT form a refined region.  DFS contains the dominance
+/* Check if two bb to bb objects BB_BB1 and BB_BB2 are equal.  */

Please s/if/that/.

+  /* A array that contains for each basic block its dominance frontier.  */

s/A array/An array/

+/* Allocate a new bb to bb object mapping from KEY to VALUE.  */
+/* Calculate the hash code of the bb to bb object BB_BB.  */
+/* Check if two bb to bb objects BB_BB1 and BB_BB2 are equal.  */

Please use some quotes around "bb to bb" or use bb-to-bb or even the
functional style notation bb->bb, as otherwise the sentence is
difficult to parse ;-)

+/* Insert an object mapping from BB to REG into REGMAP. Overwrite
+   the old value if the BB already exists.  */

Dot space space.

+static void find_regions (bitmap *dfs, htab_t bbmap)

Please put the return type of functions on a separate line.

+/* Free REGION and all its subregions.  */
+void
+free_region_tree (refined_region_p region)

Empty line between comment and function def.

+static refined_region_p
+create_region (basic_block entry, basic_block exit, refined_region_p parent,
+htab_t bbmap)

Please align under the open brace.

Could you rename bb_in_ref_region into bb_in_region, to be consistent
with the other names like region_in_region, create_region.


+/*
+What is a Region?
+================
+
+A Region is a connected subgraph of a control flow graph that has exactly two
+connections to the remaining graph.  It can be used to analyze or
optimize parts

Some of the lines in this description are too long.  Also please
indent with 3 spaces, like this:

/* What is a Region?
   ================

   A Region is a connected subgraph of a control flow graph that has
   exactly two connections to the remaining graph.  It can be used to
   analyze or optimize parts


+An refined Region (or just Region) is a subgraph that can be transformed into a

s/An refined/A refined/

+extern bool bb_in_ref_region (refined_region_p region, basic_block bb);
+extern bool region_in_region (refined_region_p outer, refined_region_p inner);
+extern void print_refined_region (FILE *F, refined_region_p region,
int indent);
+extern void debug_refined_region (refined_region_p region);

Please do not include the name of the parameter, just the type, in
function declarations.

+#ifndef GCC_GRAPHITE_REGIONS_H
+#define GCC_GRAPHITE_REGIONS_H

...

+#endif  /* GCC_GRAPHITE_H  */

This should be GCC_GRAPHITE_REGIONS_H.