diff mbox

Pragma parsing (was: [PATCH] OpenACC for C++ front end)

Message ID 87bnn1rrv3.fsf@schwinge.name
State New
Headers show

Commit Message

Thomas Schwinge Dec. 18, 2014, 1:48 p.m. UTC 
Hi!

On Thu, 13 Nov 2014 14:02:37 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> >  void
> >  init_pragma (void)
> >  {
> > +  if (flag_openacc)
> > +    {
> > +      const int n_oacc_pragmas
> > +	= sizeof (oacc_pragmas) / sizeof (*oacc_pragmas);
> > +      int i;
> > +
> > +      for (i = 0; i < n_oacc_pragmas; ++i)
> > +	cpp_register_deferred_pragma (parse_in, "acc", oacc_pragmas[i].name,
> > +				      oacc_pragmas[i].id, true, true);
> > +    }
> > +
> >    if (flag_openmp)
> >      {
> >        const int n_omp_pragmas = sizeof (omp_pragmas) / sizeof (*omp_pragmas);
> 
> Is -fopenmp -fopenacc tested not to run out of number of supported pragmas
> by libcpp?

We're running (some limited amount of) combined OpenACC/OpenMP compile
testing, yes.


> > -/* All clauses defined by OpenMP 2.5, 3.0, 3.1 and 4.0.
> > +/* All clauses defined by OpenACC 2.0, and OpenMP 2.5, 3.0, 3.1, and 4.0.
> >     Used internally by both C and C++ parsers.  */
> >  typedef enum pragma_omp_clause {
> >    PRAGMA_OMP_CLAUSE_NONE = 0,
> >  
> >    PRAGMA_OMP_CLAUSE_ALIGNED,
> > +  PRAGMA_OMP_CLAUSE_ASYNC,
> >    PRAGMA_OMP_CLAUSE_COLLAPSE,
> > +  PRAGMA_OMP_CLAUSE_COPY,
> >    PRAGMA_OMP_CLAUSE_COPYIN,
> > +  PRAGMA_OMP_CLAUSE_COPYOUT,
> >    PRAGMA_OMP_CLAUSE_COPYPRIVATE,
> > +  PRAGMA_OMP_CLAUSE_CREATE,
> >    PRAGMA_OMP_CLAUSE_DEFAULT,
> > +  PRAGMA_OMP_CLAUSE_DELETE,
> >    PRAGMA_OMP_CLAUSE_DEPEND,
> >    PRAGMA_OMP_CLAUSE_DEVICE,
> > +  PRAGMA_OMP_CLAUSE_DEVICEPTR,
> >    PRAGMA_OMP_CLAUSE_DIST_SCHEDULE,
> >    PRAGMA_OMP_CLAUSE_FINAL,
> >    PRAGMA_OMP_CLAUSE_FIRSTPRIVATE,
> >    PRAGMA_OMP_CLAUSE_FOR,
> >    PRAGMA_OMP_CLAUSE_FROM,
> > +  PRAGMA_OMP_CLAUSE_HOST,
> >    PRAGMA_OMP_CLAUSE_IF,
> >    PRAGMA_OMP_CLAUSE_INBRANCH,
> >    PRAGMA_OMP_CLAUSE_LASTPRIVATE,
> > @@ -90,16 +106,24 @@ typedef enum pragma_omp_clause {
> >    PRAGMA_OMP_CLAUSE_MERGEABLE,
> >    PRAGMA_OMP_CLAUSE_NOTINBRANCH,
> >    PRAGMA_OMP_CLAUSE_NOWAIT,
> > +  PRAGMA_OMP_CLAUSE_NUM_GANGS,
> >    PRAGMA_OMP_CLAUSE_NUM_TEAMS,
> >    PRAGMA_OMP_CLAUSE_NUM_THREADS,
> > +  PRAGMA_OMP_CLAUSE_NUM_WORKERS,
> >    PRAGMA_OMP_CLAUSE_ORDERED,
> >    PRAGMA_OMP_CLAUSE_PARALLEL,
> > +  PRAGMA_OMP_CLAUSE_PRESENT,
> > +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPY,
> > +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYIN,
> > +  PRAGMA_OMP_CLAUSE_PRESENT_OR_COPYOUT,
> > +  PRAGMA_OMP_CLAUSE_PRESENT_OR_CREATE,
> >    PRAGMA_OMP_CLAUSE_PRIVATE,
> >    PRAGMA_OMP_CLAUSE_PROC_BIND,
> >    PRAGMA_OMP_CLAUSE_REDUCTION,
> >    PRAGMA_OMP_CLAUSE_SAFELEN,
> >    PRAGMA_OMP_CLAUSE_SCHEDULE,
> >    PRAGMA_OMP_CLAUSE_SECTIONS,
> > +  PRAGMA_OMP_CLAUSE_SELF,
> >    PRAGMA_OMP_CLAUSE_SHARED,
> >    PRAGMA_OMP_CLAUSE_SIMDLEN,
> >    PRAGMA_OMP_CLAUSE_TASKGROUP,
> > @@ -107,6 +131,8 @@ typedef enum pragma_omp_clause {
> >    PRAGMA_OMP_CLAUSE_TO,
> >    PRAGMA_OMP_CLAUSE_UNIFORM,
> >    PRAGMA_OMP_CLAUSE_UNTIED,
> > +  PRAGMA_OMP_CLAUSE_VECTOR_LENGTH,
> > +  PRAGMA_OMP_CLAUSE_WAIT,
> 
> Like for CILK, I'd strongly prefer if for the clauses that are
> specific to OpenACC only you'd use PRAGMA_OACC_CLAUSE_* instead,
> and put them after the PRAGMA_CILK_* enum values.
> If you want to have PRAGMA_OACC_CLAUSE_ aliases also for the
> clauses shared in between OpenMP and OpenACC, feel free to add
> aliases like Cilk+ has them.  It is unfortunately lots of new clauses
> and we are getting close to the 64 clauses limit :( when they are used
> in bitmasks.

I still don't like this very much, because we'll then get a "mess" of
PRAGMA_OMP_CLAUSE_* intermixed with PRAGMA_OACC_CLAUSE_*.  I understand,
for example, PRAGMA_OMP_CLAUSE_REDUCTION to just be a "numeric
representation" of the "reduction" string -- and then, it doesn't make
too much sense to me to express this both as PRAGMA_OMP_CLAUSE_REDUCTION
and PRAGMA_OACC_CLAUSE_REDUCTION, or, similarly, to switch back and forth
between PRAGMA_OMP_CLAUSE_* and PRAGMA_OACC_CLAUSE_*.  Yet, that's just
the point that I'm making, so I'll defer if there's no consensus to be
found here.


Chung-Lin also had a suggestion to make; what do you think of this?



Grüße,
 Thomas

Comments

Jakub Jelinek Dec. 18, 2014, 2:29 p.m. UTC | #1 
On Thu, Dec 18, 2014 at 02:48:32PM +0100, Thomas Schwinge wrote:
> > Like for CILK, I'd strongly prefer if for the clauses that are
> > specific to OpenACC only you'd use PRAGMA_OACC_CLAUSE_* instead,
> > and put them after the PRAGMA_CILK_* enum values.
> > If you want to have PRAGMA_OACC_CLAUSE_ aliases also for the
> > clauses shared in between OpenMP and OpenACC, feel free to add
> > aliases like Cilk+ has them.  It is unfortunately lots of new clauses
> > and we are getting close to the 64 clauses limit :( when they are used
> > in bitmasks.
> 
> I still don't like this very much, because we'll then get a "mess" of
> PRAGMA_OMP_CLAUSE_* intermixed with PRAGMA_OACC_CLAUSE_*.  I understand,
> for example, PRAGMA_OMP_CLAUSE_REDUCTION to just be a "numeric
> representation" of the "reduction" string -- and then, it doesn't make
> too much sense to me to express this both as PRAGMA_OMP_CLAUSE_REDUCTION
> and PRAGMA_OACC_CLAUSE_REDUCTION, or, similarly, to switch back and forth
> between PRAGMA_OMP_CLAUSE_* and PRAGMA_OACC_CLAUSE_*.  Yet, that's just
> the point that I'm making, so I'll defer if there's no consensus to be
> found here.

The point is that we now have lots of clauses, and making it clear what from
those clauses are Cilk+, what are OpenACC, what are OpenMP will help with
code readability.  Especially if some clauses diverge in their meaning
between different standards.

> Chung-Lin also had a suggestion to make; what do you think of this?

We don't need it yet, and I'm not very keen on making GCC internals use STL
more than it does, HWI is certainly more efficient than bitset.
So I'd defer any decision here until we have to.

	Jakub
Thomas Schwinge Dec. 18, 2014, 6 p.m. UTC | #2 
Hi Jakub!

On Thu, 18 Dec 2014 15:29:42 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Dec 18, 2014 at 02:48:32PM +0100, Thomas Schwinge wrote:
> > > Like for CILK, I'd strongly prefer if for the clauses that are
> > > specific to OpenACC only you'd use PRAGMA_OACC_CLAUSE_* instead,
> > > and put them after the PRAGMA_CILK_* enum values.
> > > If you want to have PRAGMA_OACC_CLAUSE_ aliases also for the
> > > clauses shared in between OpenMP and OpenACC, feel free to add
> > > aliases like Cilk+ has them.  It is unfortunately lots of new clauses
> > > and we are getting close to the 64 clauses limit :( when they are used
> > > in bitmasks.

OK, so there is this limit.  But, I fail to understand how merely moving
the OpenACC-only PRAGMA_*_CLAUSE_* to the end of enum pragma_omp_clause
will help overcome that?  Or have I now completely confused myself, and
I'm not even understanding the problem anymore?  :-|

The only way this would make sense (in my confused mind) is, if we then
did PRAGMA_OACC_CLAUSE_REDUCTION = PRAGMA_OMP_CLAUSE_REDUCTION for the
clauses whose names exist in both standards, and
PRAGMA_OACC_CLAUSE_PRESENT = [some PRAGMA_OMP_CLAUSE_* whose name does
not exist in OpenACC], taking care that no two PRAGMA_OACC_CLAUSE_* are
assigned the same enum value.  Wouldn't this become very cumbersome for
future maintenance, however?

> > I still don't like this very much, because we'll then get a "mess" of
> > PRAGMA_OMP_CLAUSE_* intermixed with PRAGMA_OACC_CLAUSE_*.  I understand,
> > for example, PRAGMA_OMP_CLAUSE_REDUCTION to just be a "numeric
> > representation" of the "reduction" string -- and then, it doesn't make
> > too much sense to me to express this both as PRAGMA_OMP_CLAUSE_REDUCTION
> > and PRAGMA_OACC_CLAUSE_REDUCTION, or, similarly, to switch back and forth
> > between PRAGMA_OMP_CLAUSE_* and PRAGMA_OACC_CLAUSE_*.  Yet, that's just
> > the point that I'm making, so I'll defer if there's no consensus to be
> > found here.
> 
> The point is that we now have lots of clauses, and making it clear what from
> those clauses are Cilk+, what are OpenACC, what are OpenMP will help with
> code readability.

The idea then is, I guess, to split parsing of OpenACC's
PRAGMA_OACC_CLAUSE_* out of gcc/c/c-parser.c:c_parser_omp_clause_name
into a new c_parser_oacc_clause_name, just like we already have a
separate c_parser_oacc_all_clauses and c_parser_omp_all_clauses?  (The
motivation for the latter is that this is where the disambiguation
between the (possibly) different semantics of OpenACC and OpenMP clauses
happens, that is, where the "names" PRAGMA_OMP_CLAUSE_* are resolved to
actual gcc/tree-core.h:enum omp_clause_code's OMP_CLAUSE_*.  For example,
there a OpenACC copy clause is translated into a OMP_CLAUSE_MAP with
tofrom kind.)

> Especially if some clauses diverge in their meaning
> between different standards.

When you say "clauses" here, do you mean gcc/c-family/c-pragma.h:enum
pragma_omp_clause's PRAGMA_*_CLAUSE_*, or gcc/tree-core.h:enum
omp_clause_code's OMP_CLAUSE_*, or both?  That is, are you suggesting
that we should also be adding new OACC_CLAUSE_* next to the existing
OMP_CLAUSE_* in gcc/tree-core.h:enum omp_clause_code?  (Which would then
require separate handling all over the middle end?)  I thought the
(long-term) goal is to unfiy all this even more?  Or, as above, I have
now completely confused myself, and I'm not even understanding the
problem anymore?  :-|

Or, alternatively, wouldn't it also work if we did have a separate
gcc/c-family/c-pragma.h:enum pragma_oacc_clause (instead of adding to
enum pragma_omp_clause), and then the OpenACC clauses at the parsing
level live in their own "namespace"?  (The same should then be done for
Cilk+, too, even though the problem there is not as "severe" as for
OpenACC, as its only two handful PRAGMA_CILK_CLAUSE_*.)  This means we
can't support combined OpenACC/OpenMP pragmas -- but that doesn't make
much sense anyway, so no problem.  But I'm not sure that is actually the
approach you've been suggesting?


Grüße,
 Thomas
Jakub Jelinek Dec. 18, 2014, 6:06 p.m. UTC | #3 
On Thu, Dec 18, 2014 at 07:00:09PM +0100, Thomas Schwinge wrote:
> OK, so there is this limit.  But, I fail to understand how merely moving
> the OpenACC-only PRAGMA_*_CLAUSE_* to the end of enum pragma_omp_clause
> will help overcome that?  Or have I now completely confused myself, and
> I'm not even understanding the problem anymore?  :-|
> 
> The only way this would make sense (in my confused mind) is, if we then
> did PRAGMA_OACC_CLAUSE_REDUCTION = PRAGMA_OMP_CLAUSE_REDUCTION for the
> clauses whose names exist in both standards, and
> PRAGMA_OACC_CLAUSE_PRESENT = [some PRAGMA_OMP_CLAUSE_* whose name does
> not exist in OpenACC], taking care that no two PRAGMA_OACC_CLAUSE_* are
> assigned the same enum value.  Wouldn't this become very cumbersome for
> future maintenance, however?

Well, if you e.g. put PRAGMA_OMP_CLAUSE_* that isn't relevant to OpenACC
first, then pragmas shared by both, then OpenACC pragmas, then perhaps you
could use biased bitmasks for the OpenACC constructs.  Anyway, guess keep
the ordering as is.

> > > I still don't like this very much, because we'll then get a "mess" of
> > > PRAGMA_OMP_CLAUSE_* intermixed with PRAGMA_OACC_CLAUSE_*.  I understand,
> > > for example, PRAGMA_OMP_CLAUSE_REDUCTION to just be a "numeric
> > > representation" of the "reduction" string -- and then, it doesn't make
> > > too much sense to me to express this both as PRAGMA_OMP_CLAUSE_REDUCTION
> > > and PRAGMA_OACC_CLAUSE_REDUCTION, or, similarly, to switch back and forth
> > > between PRAGMA_OMP_CLAUSE_* and PRAGMA_OACC_CLAUSE_*.  Yet, that's just
> > > the point that I'm making, so I'll defer if there's no consensus to be
> > > found here.
> > 
> > The point is that we now have lots of clauses, and making it clear what from
> > those clauses are Cilk+, what are OpenACC, what are OpenMP will help with
> > code readability.
> 
> The idea then is, I guess, to split parsing of OpenACC's
> PRAGMA_OACC_CLAUSE_* out of gcc/c/c-parser.c:c_parser_omp_clause_name
> into a new c_parser_oacc_clause_name, just like we already have a
> separate c_parser_oacc_all_clauses and c_parser_omp_all_clauses?  (The
> motivation for the latter is that this is where the disambiguation
> between the (possibly) different semantics of OpenACC and OpenMP clauses
> happens, that is, where the "names" PRAGMA_OMP_CLAUSE_* are resolved to
> actual gcc/tree-core.h:enum omp_clause_code's OMP_CLAUSE_*.  For example,
> there a OpenACC copy clause is translated into a OMP_CLAUSE_MAP with
> tofrom kind.)

I don't think that is needed.  It is the same parsing, so it can be shared,
just use PRAGMA_OMP_* for OpenMP clauses or clauses shared between OpenMP
and OpenACC and PRAGMA_OACC_* for OpenACC specific pragmas in the code
that works with both, and in OpenACC specific FE code the PRAGMA_OACC_*
aliases to PRAGMA_OMP_* values for the shared clauses.
I just want to be able to clearly see what is OpenACC specific and what is
not, similarly how Cilk+ already does it.

	Jakub
Thomas Schwinge April 28, 2015, 4:59 p.m. UTC | #4 
Hi Jakub!

We now have to revisit this discussion about
gcc/c-family/c-pragma.h:pragma_omp_clause,
<http://news.gmane.org/find-root.php?message_id=%3C87bnn1rrv3.fsf%40schwinge.name%3E>:

On Thu, 18 Dec 2014 14:48:32 +0100, I wrote:
> > > -/* All clauses defined by OpenMP 2.5, 3.0, 3.1 and 4.0.
> > > +/* All clauses defined by OpenACC 2.0, and OpenMP 2.5, 3.0, 3.1, and 4.0.
> > >     Used internally by both C and C++ parsers.  */
> > >  typedef enum pragma_omp_clause {
> > >    PRAGMA_OMP_CLAUSE_NONE = 0,
> > >  
> > >    PRAGMA_OMP_CLAUSE_ALIGNED,
> > > [...]
> > >    PRAGMA_OMP_CLAUSE_TO,
> > >    PRAGMA_OMP_CLAUSE_UNIFORM,
> > >    PRAGMA_OMP_CLAUSE_UNTIED,
> > > +  PRAGMA_OMP_CLAUSE_VECTOR_LENGTH,
> > > +  PRAGMA_OMP_CLAUSE_WAIT,
> > 
> > Like for CILK, I'd strongly prefer if for the clauses that are
> > specific to OpenACC only you'd use PRAGMA_OACC_CLAUSE_* instead,
> > and put them after the PRAGMA_CILK_* enum values.
> > If you want to have PRAGMA_OACC_CLAUSE_ aliases also for the
> > clauses shared in between OpenMP and OpenACC, feel free to add
> > aliases like Cilk+ has them.

(That's what we ended up doing.)

> > It is unfortunately lots of new clauses
> > and we are getting close to the 64 clauses limit :( when they are used
> > in bitmasks.

We're getting ready to submit patches extending the C/C++ front ends for
the remaining OpenACC clauses, and given the current layout of
gcc/c-family/c-pragma.h:pragma_omp_clause, we're then at 69 unique
clauses, which is more than the 64-bit bitmask limit.

> [...] I understand,
> for example, PRAGMA_OMP_CLAUSE_REDUCTION to just be a "numeric
> representation" of the "reduction" string [...]

Should we now further split parsing of Cilk+/OpenACC/OpenMP pragmas (but
I think you and I agreed that we don't want to go that route), or
revisist Chung-Lin's std::bitset proposal (see below), or something else?

Regarding std::bitset, you've been worried about performance,
<http://news.gmane.org/find-root.php?message_id=%3C20141218142942.GM1667%40tucnak.redhat.com%3E>,
but I'm not sure about this: aren't these code paths only exercised for
pragma parsing, and aren't there really only ever one handful (or, a few)
of pragmas per source code file, so this could hardly be an observable
performance hit?

> Chung-Lin also had a suggestion to make; what do you think of this?
> 
> --- c-family/c-common.h	(revision 442892)
> +++ c-family/c-common.h	(working copy)
> @@ -1076,138 +1076,17 @@ extern void pp_dir_change (cpp_reader *, const cha
>  extern bool check_missing_format_attribute (tree, tree);
>  
>  /* In c-omp.c  */
> -#if HOST_BITS_PER_WIDE_INT >= 64
> -typedef unsigned HOST_WIDE_INT omp_clause_mask;
> -# define OMP_CLAUSE_MASK_1 ((omp_clause_mask) 1)
> -#else
> -struct omp_clause_mask
> +#include <bitset>
> +typedef std::bitset<128> omp_clause_mask;
> +/* Provide '&' before full transition to set() method.  */
> +static inline omp_clause_mask
> +operator & (const omp_clause_mask& a, const omp_clause_mask& b)
>  {
> -  inline omp_clause_mask ();
> -  inline omp_clause_mask (unsigned HOST_WIDE_INT l);
> -  inline omp_clause_mask (unsigned HOST_WIDE_INT l,
> -			  unsigned HOST_WIDE_INT h);
> -  inline omp_clause_mask &operator &= (omp_clause_mask);
> -  inline omp_clause_mask &operator |= (omp_clause_mask);
> -  inline omp_clause_mask operator ~ () const;
> -  inline omp_clause_mask operator & (omp_clause_mask) const;
> -  inline omp_clause_mask operator | (omp_clause_mask) const;
> -  inline omp_clause_mask operator >> (int);
> -  inline omp_clause_mask operator << (int);
> -  inline bool operator == (omp_clause_mask) const;
> -  inline bool operator != (omp_clause_mask) const;
> -  unsigned HOST_WIDE_INT low, high;
> -};
> -
> -inline
> -omp_clause_mask::omp_clause_mask ()
> -{
> +  omp_clause_mask tmp = a;
> +  return (tmp &= b);
>  }
> +#define OMP_CLAUSE_MASK_1 (omp_clause_mask (1))
>  
> -inline
> -omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l)
> -: low (l), high (0)
> -{
> -}
> -
> -inline
> -omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l,
> -				  unsigned HOST_WIDE_INT h)
> -: low (l), high (h)
> -{
> -}
> -
> -inline omp_clause_mask &
> -omp_clause_mask::operator &= (omp_clause_mask b)
> -{
> -  low &= b.low;
> -  high &= b.high;
> -  return *this;
> -}
> -
> -inline omp_clause_mask &
> -omp_clause_mask::operator |= (omp_clause_mask b)
> -{
> -  low |= b.low;
> -  high |= b.high;
> -  return *this;
> -}
> -
> -inline omp_clause_mask
> -omp_clause_mask::operator ~ () const
> -{
> -  omp_clause_mask ret (~low, ~high);
> -  return ret;
> -}
> -
> -inline omp_clause_mask
> -omp_clause_mask::operator | (omp_clause_mask b) const
> -{
> -  omp_clause_mask ret (low | b.low, high | b.high);
> -  return ret;
> -}
> -
> -inline omp_clause_mask
> -omp_clause_mask::operator & (omp_clause_mask b) const
> -{
> -  omp_clause_mask ret (low & b.low, high & b.high);
> -  return ret;
> -}
> -
> -inline omp_clause_mask
> -omp_clause_mask::operator << (int amount)
> -{
> -  omp_clause_mask ret;
> -  if (amount >= HOST_BITS_PER_WIDE_INT)
> -    {
> -      ret.low = 0;
> -      ret.high = low << (amount - HOST_BITS_PER_WIDE_INT);
> -    }
> -  else if (amount == 0)
> -    ret = *this;
> -  else
> -    {
> -      ret.low = low << amount;
> -      ret.high = (low >> (HOST_BITS_PER_WIDE_INT - amount))
> -		 | (high << amount);
> -    }
> -  return ret;
> -}
> -
> -inline omp_clause_mask
> -omp_clause_mask::operator >> (int amount)
> -{
> -  omp_clause_mask ret;
> -  if (amount >= HOST_BITS_PER_WIDE_INT)
> -    {
> -      ret.low = high >> (amount - HOST_BITS_PER_WIDE_INT);
> -      ret.high = 0;
> -    }
> -  else if (amount == 0)
> -    ret = *this;
> -  else
> -    {
> -      ret.low = (high << (HOST_BITS_PER_WIDE_INT - amount))
> -		 | (low >> amount);
> -      ret.high = high >> amount;
> -    }
> -  return ret;
> -}
> -
> -inline bool
> -omp_clause_mask::operator == (omp_clause_mask b) const
> -{
> -  return low == b.low && high == b.high;
> -}
> -
> -inline bool
> -omp_clause_mask::operator != (omp_clause_mask b) const
> -{
> -  return low != b.low || high != b.high;
> -}
> -
> -# define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
> -#endif
> -
>  enum c_omp_clause_split
>  {
>    C_OMP_CLAUSE_SPLIT_TARGET = 0,


Grüße,
 Thomas
Jakub Jelinek April 28, 2015, 5:16 p.m. UTC | #5 
On Tue, Apr 28, 2015 at 06:59:12PM +0200, Thomas Schwinge wrote:
> We're getting ready to submit patches extending the C/C++ front ends for
> the remaining OpenACC clauses, and given the current layout of
> gcc/c-family/c-pragma.h:pragma_omp_clause, we're then at 69 unique
> clauses, which is more than the 64-bit bitmask limit.

I have an uncommitted patch for that that I plan to commit to
gomp-4_1-branch tomorrow.
Guess I should extract it and commit to trunk separately.

	Jakub
diff mbox

Patch

--- c-family/c-common.h	(revision 442892)
+++ c-family/c-common.h	(working copy)
@@ -1076,138 +1076,17 @@  extern void pp_dir_change (cpp_reader *, const cha
 extern bool check_missing_format_attribute (tree, tree);
 
 /* In c-omp.c  */
-#if HOST_BITS_PER_WIDE_INT >= 64
-typedef unsigned HOST_WIDE_INT omp_clause_mask;
-# define OMP_CLAUSE_MASK_1 ((omp_clause_mask) 1)
-#else
-struct omp_clause_mask
+#include <bitset>
+typedef std::bitset<128> omp_clause_mask;
+/* Provide '&' before full transition to set() method.  */
+static inline omp_clause_mask
+operator & (const omp_clause_mask& a, const omp_clause_mask& b)
 {
-  inline omp_clause_mask ();
-  inline omp_clause_mask (unsigned HOST_WIDE_INT l);
-  inline omp_clause_mask (unsigned HOST_WIDE_INT l,
-			  unsigned HOST_WIDE_INT h);
-  inline omp_clause_mask &operator &= (omp_clause_mask);
-  inline omp_clause_mask &operator |= (omp_clause_mask);
-  inline omp_clause_mask operator ~ () const;
-  inline omp_clause_mask operator & (omp_clause_mask) const;
-  inline omp_clause_mask operator | (omp_clause_mask) const;
-  inline omp_clause_mask operator >> (int);
-  inline omp_clause_mask operator << (int);
-  inline bool operator == (omp_clause_mask) const;
-  inline bool operator != (omp_clause_mask) const;
-  unsigned HOST_WIDE_INT low, high;
-};
-
-inline
-omp_clause_mask::omp_clause_mask ()
-{
+  omp_clause_mask tmp = a;
+  return (tmp &= b);
 }
+#define OMP_CLAUSE_MASK_1 (omp_clause_mask (1))
 
-inline
-omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l)
-: low (l), high (0)
-{
-}
-
-inline
-omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l,
-				  unsigned HOST_WIDE_INT h)
-: low (l), high (h)
-{
-}
-
-inline omp_clause_mask &
-omp_clause_mask::operator &= (omp_clause_mask b)
-{
-  low &= b.low;
-  high &= b.high;
-  return *this;
-}
-
-inline omp_clause_mask &
-omp_clause_mask::operator |= (omp_clause_mask b)
-{
-  low |= b.low;
-  high |= b.high;
-  return *this;
-}
-
-inline omp_clause_mask
-omp_clause_mask::operator ~ () const
-{
-  omp_clause_mask ret (~low, ~high);
-  return ret;
-}
-
-inline omp_clause_mask
-omp_clause_mask::operator | (omp_clause_mask b) const
-{
-  omp_clause_mask ret (low | b.low, high | b.high);
-  return ret;
-}
-
-inline omp_clause_mask
-omp_clause_mask::operator & (omp_clause_mask b) const
-{
-  omp_clause_mask ret (low & b.low, high & b.high);
-  return ret;
-}
-
-inline omp_clause_mask
-omp_clause_mask::operator << (int amount)
-{
-  omp_clause_mask ret;
-  if (amount >= HOST_BITS_PER_WIDE_INT)
-    {
-      ret.low = 0;
-      ret.high = low << (amount - HOST_BITS_PER_WIDE_INT);
-    }
-  else if (amount == 0)
-    ret = *this;
-  else
-    {
-      ret.low = low << amount;
-      ret.high = (low >> (HOST_BITS_PER_WIDE_INT - amount))
-		 | (high << amount);
-    }
-  return ret;
-}
-
-inline omp_clause_mask
-omp_clause_mask::operator >> (int amount)
-{
-  omp_clause_mask ret;
-  if (amount >= HOST_BITS_PER_WIDE_INT)
-    {
-      ret.low = high >> (amount - HOST_BITS_PER_WIDE_INT);
-      ret.high = 0;
-    }
-  else if (amount == 0)
-    ret = *this;
-  else
-    {
-      ret.low = (high << (HOST_BITS_PER_WIDE_INT - amount))
-		 | (low >> amount);
-      ret.high = high >> amount;
-    }
-  return ret;
-}
-
-inline bool
-omp_clause_mask::operator == (omp_clause_mask b) const
-{
-  return low == b.low && high == b.high;
-}
-
-inline bool
-omp_clause_mask::operator != (omp_clause_mask b) const
-{
-  return low != b.low || high != b.high;
-}
-
-# define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
-#endif
-
 enum c_omp_clause_split
 {
   C_OMP_CLAUSE_SPLIT_TARGET = 0,