gcc/c-family/c-pragma.h:pragma_omp_clause (was: Pragma parsing)

Jakub Jelinek jakub@redhat.com
Wed Apr 29 08:50:00 GMT 2015


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 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?

As I said earlier, I run into this during OpenMP 4.1 work too.  The
std::bitset variant generates significantly larger code to what I've
bootstrapped/regtested on x86_64-linux and i686-linux and committed to
trunk.  Furthermore, there are severe issues with including STL headers
in the middle of GCC include files, and not even C++14 allows bitsets to be
readably initialized (...set(X).set(Y).set(Z).set(W) doesn't IMHO count).

2015-04-29  Jakub Jelinek  <jakub@redhat.com>

	* c-common.h (omp_clause_mask): Unconditionally define as a class.
	Use uint64_t instead of unsigned HOST_WIDE_INT and 64 instead of
	HOST_BITS_PER_WIDE_INT.

--- gcc/c-family/c-common.h.jj	2015-04-11 15:55:57.000000000 +0200
+++ gcc/c-family/c-common.h	2015-04-29 10:19:17.629643284 +0200
@@ -1096,16 +1096,11 @@ extern void pp_dir_change (cpp_reader *,
 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
 {
   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 (uint64_t l);
+  inline omp_clause_mask (uint64_t l, uint64_t h);
   inline omp_clause_mask &operator &= (omp_clause_mask);
   inline omp_clause_mask &operator |= (omp_clause_mask);
   inline omp_clause_mask operator ~ () const;
@@ -1115,7 +1110,7 @@ struct omp_clause_mask
   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;
+  uint64_t low, high;
 };
 
 inline
@@ -1124,14 +1119,13 @@ omp_clause_mask::omp_clause_mask ()
 }
 
 inline
-omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l)
+omp_clause_mask::omp_clause_mask (uint64_t l)
 : low (l), high (0)
 {
 }
 
 inline
-omp_clause_mask::omp_clause_mask (unsigned HOST_WIDE_INT l,
-				  unsigned HOST_WIDE_INT h)
+omp_clause_mask::omp_clause_mask (uint64_t l, uint64_t h)
 : low (l), high (h)
 {
 }
@@ -1177,18 +1171,17 @@ inline omp_clause_mask
 omp_clause_mask::operator << (int amount)
 {
   omp_clause_mask ret;
-  if (amount >= HOST_BITS_PER_WIDE_INT)
+  if (amount >= 64)
     {
       ret.low = 0;
-      ret.high = low << (amount - HOST_BITS_PER_WIDE_INT);
+      ret.high = low << (amount - 64);
     }
   else if (amount == 0)
     ret = *this;
   else
     {
       ret.low = low << amount;
-      ret.high = (low >> (HOST_BITS_PER_WIDE_INT - amount))
-		 | (high << amount);
+      ret.high = (low >> (64 - amount)) | (high << amount);
     }
   return ret;
 }
@@ -1197,17 +1190,16 @@ inline omp_clause_mask
 omp_clause_mask::operator >> (int amount)
 {
   omp_clause_mask ret;
-  if (amount >= HOST_BITS_PER_WIDE_INT)
+  if (amount >= 64)
     {
-      ret.low = high >> (amount - HOST_BITS_PER_WIDE_INT);
+      ret.low = high >> (amount - 64);
       ret.high = 0;
     }
   else if (amount == 0)
     ret = *this;
   else
     {
-      ret.low = (high << (HOST_BITS_PER_WIDE_INT - amount))
-		 | (low >> amount);
+      ret.low = (high << (64 - amount)) | (low >> amount);
       ret.high = high >> amount;
     }
   return ret;
@@ -1225,8 +1217,7 @@ omp_clause_mask::operator != (omp_clause
   return low != b.low || high != b.high;
 }
 
-# define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
-#endif
+#define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
 
 enum c_omp_clause_split
 {


	Jakub



More information about the Gcc-patches mailing list