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: [Gomp] openmp syntax check - trial approach


Hi Diego,

Good to hear from you. Thanks a lot for the very useful comments.
I have modified my patch locally to reflect most your comments. As
to the lacking calls to gomp_nyi my initial idea was only to
do pure openMP pragma syntax checking and to postpone the 
semantics completely for now, i.e. the call to gomp_nyi is
missing in ALL the places where you have a completely parsed 
openMP pragma :) As to DECL_P or not, these lines stems from the 
original c-gomp.c file and I haven't considered whether or not 
DECL_P should be added.

Finally, I do not have write access to CVS but maybe we shouldn't
rush that for now. I don't mind sending you my patches while we
are waiting for a more complete design draft.  Speaking of which. 
Have you had time to glance the ompnote that Ross posted a long time ago
(http://lists.gnu.org/archive/html/gomp-discuss/2004-08/msg00000.html.)
I did play a bit with that idea the other day, cf. 
http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01146.html,
and I would like to know how hard it would be based on the line

#pragma omp parallel for 

to 

- check that the next block is indeed a for-loop
- get the index variable (iv) for the loop and its type (t_iv)
- get the upper and lower bounds and the step-size for the loop.
- get the rest of the variables (v1, v2, ...) involved in the loop and 
  their type (t_v1, t_v2,...)

allowing us to construct a struct (and add it to the code)
containing the variables for the loop:

struct _args {
	t_v1 v1;
	t_v2 v2;
	...
};

and construct a function (and add it to the code):

static void *_region(void *args) {
	_OMP_Thread_Args *ta;
	struct _args *ra;
	t_v1 v1;
	t_v2 v2;
	t_iv _chunk_lower, _chunk_upper, iv;

	ta = (_OMP_Thread_Args *) args;
	ra = (struct _args *) ta->args;
	v1 = ra->v1;
	v2 = ra->v2;
	...

}

I would suspect that we should start in the gomp_handle_clause() in
c-gomp.c but what should we actually do here and what should we simply
mark up for later handling. I would like to look into it if someone
knowing the internals of gcc could give me some directions. Knowing 
how "hard"/"easy" it would be to do the translation will hopefully give 
us a good idea on whether or not we should continue to work on the 
proposed design or we should drop it for something else.

Cheers, Jacob

* Diego Novillo <dnovillo@redhat.com> [2005-01-19 14:57]:
> Jacob Weismann Poulsen wrote:
> 
> >The patch allows one to do syntax checking on all the samples in the
> >appendix of the openMP specification version 2.0 BUT it should be mentioned
> >that I haven't polished it nor tested it properly yet (better get some 
> >reactions first).
> >
> This is a good start.  Good stuff.
> 
> I would probably keep with the convention of calling these routines 
> c_gomp_...
> 
> Other than that, the patch looks reasonable (some suggestions below). 
> You will need ChangeLog entries.  Do you have write access to CVS?
> 
> >-/* Return true when fails to find an open parenthesis.  */
> >+/* Return false when fails to find new-line.  */
> > 
> s/when fails/when it fails/
> 
> >-      error ("Expected an open parenthesis.");
> >-      return true;
> >+      error ("Expected a newline in handling of '#pragma omp'.");
> >
> The convention is not to end error messages with '.'
> 
> 
> >+  while (token == CPP_NAME)
> >+    {
> >+      if (name_t == NULL_TREE
> >+	  || (name_decl = lookup_name (name_t)) == NULL_TREE
> >+	  || TREE_CODE (name_decl) != VAR_DECL)
> >
> Maybe DECL_P?  Are function arguments valid here?
> 
> >+	{
> >+	  inform ("Expected a variable.");
> >+	}
> >+      /* else TODO: mark that variable as private...  */
> >+      token = c_lex (&name_t);
> >+      if (token == CPP_COMMA)
> >+	{
> >+	  token = c_lex (&name_t);
> >+	  if (token == CPP_NAME)
> >+	    {
> >+	      if (name_t == NULL_TREE
> >+		  || (name_decl = lookup_name (name_t)) == NULL_TREE
> >+		  || TREE_CODE (name_decl) != VAR_DECL)
> >
> Likewise.  This predicate could be factored.
> 
> >+/* Parses the set of possible OMP clauses */
> >+
> >+static void
> >+gomp_handle_clause (enum omp_clause clause)
> >
> Document the argument.  Mention it in capitals.
> 
> >+	  const char *name = IDENTIFIER_POINTER (x);
> >+	  if (!strcmp (name, "nowait"))
> >+	    {
> >+	    }
> >+
> call gomp_nyi here?
> 
> >+		      const char *name = IDENTIFIER_POINTER (x);
> >+		      if (!strcmp (name, "none"))
> >+			{
> >+			}
> >
> Likewise.
> 
> >+		      else if (!strcmp (name, "shared"))
> >+			{
> >+			}
> >
> Likewise.
> 
> >+	  else if (!strcmp (name, "reduction"))
> >+	    {
> >+	      gomp_handle_reduction ();
> >+	    }
> >
> No need to use braces when the body is a single line.  But that's a 
> matter of personal preference.
> 
> >+/* Parse a parallel-directive:
> >+ * "#pragma omp parallel [par_clause[ [, par_clause] ... ]] new-line".  */
> > 
> > void
> >-c_gomp_parallel_pragma (cpp_reader *pfile)
> >+c_gomp_parallel_pragma (cpp_reader * pfile)
> 
> Document arguments.
> 
> 
> >-c_gomp_flush_pragma (cpp_reader *pfile ATTRIBUTE_UNUSED)
> >+c_gomp_flush_pragma (cpp_reader * pfile ATTRIBUTE_UNUSED)
> >
> No space after '*'.
> 
> 
> 
> Diego.

-- 
Jacob Weismann Poulsen <jwp@dmi.dk>
Fingerprint: 9315 DC43 D2E4 4F70 3AA8  F8F0 9DA0 B765 F5C8 7D26


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