This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Autoparallelization
- From: Diego Novillo <dnovillo at google dot com>
- To: Zdenek Dvorak <rakdver at kam dot mff dot cuni dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 13 Sep 2007 14:22:02 -0400
- Subject: Re: [patch] Autoparallelization
- References: <20070909134713.GA31281@kam.mff.cuni.cz>
On 9-Sep-07, at 09:47 , Zdenek Dvorak wrote:
+ @item -ftree-parallelize-loops=n
+ @opindex ftree-parallelize-loops=n
+ Parallelize loops, i.e., split their iteration space to run in n
threads.
+
Add a sentence or two describing how the cost model works and what
types of loops are expected to be parallelized.
+ /* Parallelization. */
+
+ static bool
+ gate_tree_parallelize_loops (void)
+ static unsigned
+ tree_parallelize_loops (void)
+ struct tree_opt_pass pass_parallelize_loops =
Why not declare these three in tree-parloops.c? Not that it matters
much, just curious.
+
+ /* The iterations of the loop may comunicate only through bivs
whose
s/comunicate/communicate/
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, " FAILED: non-biv phi node\n");
Hmm, these messages are typically read by users who are looking to
parallelize their code. The phrase 'non-biv phi node' will be
perplexing to anyone not thoroughly familiar with compiler lingo.
+
+ /* Check for problems with dependences. If the loop can be
reversed,
+ the iterations are indepent. */
s/indepent/independent/
+ else if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, " FAILED: dependence check failed\n");
Similarly, how about something like "data dependencies exist across
iterations" or "iterations are not independent of each other".
+ /* Moves the references to local variables in STMT from LOOP.
DECL_ADDRESS
s/from/out of/
+ contains addresses for the references for that we have already
taken
+ them. */
Not sure what you wanted to say here. Perhaps, "contains addresses
of the references that had their address taken already"?
+ /* Moves all the variables used in LOOP and defined outside of it
(including
+ the initial values of loop phi nodes, and *PER_THREAD if it is
a ssa
+ name) to a structure created for this purpose. The code
+
+ while (1)
+ {
+ use (a);
+ use (b);
+ }
+
+ is transformed this way:
+
+ bb0:
+ old.a = a;
+ old.b = b;
+
+ bb1:
+ a' = new->a;
+ b' = new->b;
+ while (1)
+ {
+ use (a');
+ use (b');
+ }
Watch indentation here.
+ /* We should mark *arg_struct call clobbered. However,
this means
+ we would need to update virtual operands for every function call.
+ To avoid this, we pretend this variable is volatile. */
+ TREE_THIS_VOLATILE (*arg_struct) = 1;
Hmm, OK I guess. Not too happy about this, but I understand why
you'd want to do this.
+
+ /* Creates and returns an empty function to that the body of the
loop
+ is later split. */
Not sure what you're trying to say here. Maybe "... an empty
function that will receive the body of a parallelized loop"?
+ /* Emit OMP_FOR. */
+ TREE_OPERAND (cond, 0) = cvar_base;
+ type = TREE_TYPE (cvar);
+ t = build_omp_clause (OMP_CLAUSE_SCHEDULE);
+ OMP_CLAUSE_SCHEDULE_KIND (t) = OMP_CLAUSE_SCHEDULE_STATIC;
Do we want to specify this as a --param? I can see future
improvements where we allow the user to determine what scheduling to
use and whatnot. The patch looks like it would support different
loop schedulings, or is there something I'm missing?
The rest looks fine. Thanks for this feature!