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: [buildrobot] OMP: r203408 probably needs another operator& returning bool


On Fri, Oct 11, 2013 at 10:11:21AM -0400, Jason Merrill wrote:
> >another
> >possibility is not to add operator bool () overload that introduces that
> >ambiguity, but then if (mask & something) needs to be replaced with
> >if ((mask & something) != 0) and operator != (int) added.
> >I guess I slightly prefer the first patch since it is smaller.
> 
> Since the coding standards say "Conversion operators should be
> avoided" (because they can't be explicit), I think this is the way
> to go.

We then violate the coding standard in vec.h:
/* Type to provide NULL values for vec<T, A, L>.  This is used to
   provide nil initializers for vec instances.  Since vec must be
   a POD, we cannot have proper ctor/dtor for it.  To initialize
   a vec instance, you can assign it the value vNULL.  */
struct vnull
{
  template <typename T, typename A, typename L>
  operator vec<T, A, L> () { return vec<T, A, L>(); }
};
extern vnull vNULL;

but perhaps we can keep that as an exception and don't allow further
exceptions...

> But it would be better to add operator!=(omp_clause_mask).

Thanks, that works too, thus here is what I've committed.

2013-10-11  Jakub Jelinek  <jakub@redhat.com>

	* c-common.h (omp_clause_mask::operator !=): New method.
	* c-omp.c (c_omp_split_clauses): Use if ((mask & something) != 0)
	instead of if (mask & something) tests everywhere.

--- gcc/c-family/c-common.h.jj	2013-10-11 16:14:35.157176208 +0200
+++ gcc/c-family/c-common.h	2013-10-11 16:15:30.031891445 +0200
@@ -1052,6 +1052,7 @@ struct omp_clause_mask
   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;
 };
 
@@ -1156,6 +1157,12 @@ omp_clause_mask::operator == (omp_clause
   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
 
--- gcc/c-family/c-omp.c.jj	2013-10-11 16:14:35.153176228 +0200
+++ gcc/c-family/c-omp.c	2013-10-11 16:14:48.164106509 +0200
@@ -631,7 +631,7 @@ c_omp_split_clauses (location_t loc, enu
     cclauses[i] = NULL;
   /* Add implicit nowait clause on
      #pragma omp parallel {for,for simd,sections}.  */
-  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) != 0)
     switch (code)
       {
       case OMP_FOR:
@@ -691,10 +691,10 @@ c_omp_split_clauses (location_t loc, enu
 	      OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_SIMD];
 	      cclauses[C_OMP_CLAUSE_SPLIT_SIMD] = c;
 	    }
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0)
 	    {
-	      if (mask & (OMP_CLAUSE_MASK_1
-			  << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE))
+	      if ((mask & (OMP_CLAUSE_MASK_1
+			   << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) != 0)
 		{
 		  c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
 					OMP_CLAUSE_COLLAPSE);
@@ -729,19 +729,20 @@ c_omp_split_clauses (location_t loc, enu
 	   target and simd.  Put it on the outermost of those and
 	   duplicate on parallel.  */
 	case OMP_CLAUSE_FIRSTPRIVATE:
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	      != 0)
 	    {
-	      if (mask & ((OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)
-			  | (OMP_CLAUSE_MASK_1
-			     << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)))
+	      if ((mask & ((OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)
+			   | (OMP_CLAUSE_MASK_1
+			      << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE))) != 0)
 		{
 		  c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
 					OMP_CLAUSE_FIRSTPRIVATE);
 		  OMP_CLAUSE_DECL (c) = OMP_CLAUSE_DECL (clauses);
 		  OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL];
 		  cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL] = c;
-		  if (mask & (OMP_CLAUSE_MASK_1
-			      << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+		  if ((mask & (OMP_CLAUSE_MASK_1
+			       << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) != 0)
 		    s = C_OMP_CLAUSE_SPLIT_TEAMS;
 		  else
 		    s = C_OMP_CLAUSE_SPLIT_DISTRIBUTE;
@@ -751,14 +752,15 @@ c_omp_split_clauses (location_t loc, enu
 		   #pragma omp parallel{, for{, simd}, sections}.  */
 		s = C_OMP_CLAUSE_SPLIT_PARALLEL;
 	    }
-	  else if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+	  else if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+		   != 0)
 	    {
 	      /* This must be #pragma omp {,target }teams distribute.  */
 	      gcc_assert (code == OMP_DISTRIBUTE);
 	      s = C_OMP_CLAUSE_SPLIT_TEAMS;
 	    }
-	  else if (mask & (OMP_CLAUSE_MASK_1
-			   << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE))
+	  else if ((mask & (OMP_CLAUSE_MASK_1
+			    << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) != 0)
 	    {
 	      /* This must be #pragma omp distribute simd.  */
 	      gcc_assert (code == OMP_SIMD);
@@ -777,19 +779,21 @@ c_omp_split_clauses (location_t loc, enu
 	case OMP_CLAUSE_LASTPRIVATE:
 	  if (code == OMP_FOR || code == OMP_SECTIONS)
 	    {
-	      if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	      if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+		  != 0)
 		s = C_OMP_CLAUSE_SPLIT_PARALLEL;
 	      else
 		s = C_OMP_CLAUSE_SPLIT_FOR;
 	      break;
 	    }
 	  gcc_assert (code == OMP_SIMD);
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0)
 	    {
 	      c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
 				    OMP_CLAUSE_LASTPRIVATE);
 	      OMP_CLAUSE_DECL (c) = OMP_CLAUSE_DECL (clauses);
-	      if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	      if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+		  != 0)
 		s = C_OMP_CLAUSE_SPLIT_PARALLEL;
 	      else
 		s = C_OMP_CLAUSE_SPLIT_FOR;
@@ -806,7 +810,8 @@ c_omp_split_clauses (location_t loc, enu
 	      s = C_OMP_CLAUSE_SPLIT_TEAMS;
 	      break;
 	    }
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+	      != 0)
 	    {
 	      c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
 				    OMP_CLAUSE_CODE (clauses));
@@ -837,9 +842,10 @@ c_omp_split_clauses (location_t loc, enu
 	      OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_SIMD];
 	      cclauses[C_OMP_CLAUSE_SPLIT_SIMD] = c;
 	    }
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0)
 	    {
-	      if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+	      if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS))
+		  != 0)
 		{
 		  c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
 					OMP_CLAUSE_REDUCTION);
@@ -852,8 +858,8 @@ c_omp_split_clauses (location_t loc, enu
 		  cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL] = c;
 		  s = C_OMP_CLAUSE_SPLIT_TEAMS;
 		}
-	      else if (mask & (OMP_CLAUSE_MASK_1
-			       << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	      else if ((mask & (OMP_CLAUSE_MASK_1
+				<< PRAGMA_OMP_CLAUSE_NUM_THREADS)) != 0)
 		s = C_OMP_CLAUSE_SPLIT_PARALLEL;
 	      else
 		s = C_OMP_CLAUSE_SPLIT_FOR;
@@ -865,7 +871,8 @@ c_omp_split_clauses (location_t loc, enu
 	  break;
 	case OMP_CLAUSE_IF:
 	  /* FIXME: This is currently being discussed.  */
-	  if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
+	      != 0)
 	    s = C_OMP_CLAUSE_SPLIT_PARALLEL;
 	  else
 	    s = C_OMP_CLAUSE_SPLIT_TARGET;


	Jakub


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