This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Autoparallelization



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!



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]