Hi!
On Fri, Oct 11, 2013 at 02:44:16PM +0200, Jan-Benedict Glaw wrote:
> The recent change probably gave us this[1]:
>
> g++ -c -DIN_GCC_FRONTEND -DIN_GCC_FRONTEND -g -O2 -DIN_GCC
> -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti
> -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
> -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long
> -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I.
> -Ic-family -I../../../../gcc/gcc -I../../../../gcc/gcc/c-family
> -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include
> -I../../../../gcc/gcc/../libdecnumber
> -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
> -I../../../../gcc/gcc/../libbacktrace -o c-family/c-omp.o -MT
> c-family/c-omp.o -MMD -MP -MF c-family/.deps/c-omp.TPo
> ../../../../gcc/gcc/c-family/c-omp.c
> ../../../../gcc/gcc/c-family/c-omp.c: In function ‘void
> c_omp_split_clauses(location_t, tree_code, omp_clause_mask, tree,
> tree_node**)’:
> ../../../../gcc/gcc/c-family/c-omp.c:634:12: error: could not convert
> ‘mask.omp_clause_mask::operator&(omp_clause_mask(1ul).omp_clause_mask::operator<<(22))’
> from ‘omp_clause_mask’ to ‘bool’
> if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS))
> ^
The original omp_clause_mask fallback was tested, but since then several
changes have been made and I forgot to retest it; when HOST_WIDE_INT is
64-bit omp_clause_mask is a normal unsigned HOST_WIDE_INT and no C++
stuff is used to emulate 64-bit bitmask.
I have tested two different versions of a fix for this, dunno which one is
preferrable, Jason?
With the operator bool (), there is ambiguity in the
if (((mask >> something) & 1) == 0)
tests (so had to use OMP_CLAUSE_MASK_{1,0} instead of {1,0}), 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.
Jakub
2013-10-11 Jakub Jelinek <ja...@redhat.com>
c-family/
* c-common.h (OMP_CLAUSE_MASK_0): Define.
(omp_clause_mask::operator bool ()): New method.
c/
* c-parser.c (c_parser_omp_all_clauses): Use OMP_CLAUSE_MASK_1
and OMP_CLAUSE_MASK_0 instead of 1 and 0 to avoid ambiguity.
cp/
* parser.c (cp_parser_omp_all_clauses): Use OMP_CLAUSE_MASK_1
and OMP_CLAUSE_MASK_0 instead of 1 and 0 to avoid ambiguity.
--- gcc/c-family/c-common.h.jj 2013-10-11 11:23:59.000000000 +0200
+++ gcc/c-family/c-common.h 2013-10-11 15:40:32.581791018 +0200
@@ -1036,6 +1036,7 @@ extern void pp_dir_change (cpp_reader *,
/* In c-omp.c */
#if HOST_BITS_PER_WIDE_INT >= 64
typedef unsigned HOST_WIDE_INT omp_clause_mask;
+# define OMP_CLAUSE_MASK_0 ((omp_clause_mask) 0)
# define OMP_CLAUSE_MASK_1 ((omp_clause_mask) 1)
#else
struct omp_clause_mask
@@ -1052,6 +1053,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 operator bool () const;
unsigned HOST_WIDE_INT low, high;
};
@@ -1156,6 +1158,13 @@ omp_clause_mask::operator == (omp_clause
return low == b.low && high == b.high;
}
+inline
+omp_clause_mask::operator bool () const
+{
+ return low || high;
+}
+
+# define OMP_CLAUSE_MASK_0 omp_clause_mask (0)
# define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
#endif
--- gcc/c/c-parser.c.jj 2013-10-11 11:23:59.000000000 +0200
+++ gcc/c/c-parser.c 2013-10-11 15:41:35.864464738 +0200
@@ -10644,7 +10644,8 @@ c_parser_omp_all_clauses (c_parser *pars
first = false;
- if (((mask >> c_kind) & 1) == 0 && !parser->error)
+ if (((mask >> c_kind) & OMP_CLAUSE_MASK_1) == OMP_CLAUSE_MASK_0
+ && !parser->error)
{
/* Remove the invalid clause(s) from the list to avoid
confusing the rest of the compiler. */
--- gcc/cp/parser.c.jj 2013-10-11 11:23:59.000000000 +0200
+++ gcc/cp/parser.c 2013-10-11 15:40:50.939719303 +0200
@@ -27865,7 +27865,7 @@ cp_parser_omp_all_clauses (cp_parser *pa
first = false;
- if (((mask >> c_kind) & 1) == 0)
+ if (((mask >> c_kind) & OMP_CLAUSE_MASK_1) == OMP_CLAUSE_MASK_0)
{
/* Remove the invalid clause(s) from the list to avoid
confusing the rest of the compiler. */
2013-10-11 Jakub Jelinek <ja...@redhat.com>
* c-common.h (omp_clause_mask::operator != (int)): 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 11:23:59.000000000 +0200
+++ gcc/c-family/c-common.h 2013-10-11 15:33:12.954073363 +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 != (int) const;
unsigned HOST_WIDE_INT low, high;
};
@@ -1156,6 +1157,16 @@ omp_clause_mask::operator == (omp_clause
return low == b.low && high == b.high;
}
+inline bool
+omp_clause_mask::operator != (int b) const
+{
+ if (low != (unsigned HOST_WIDE_INT) b)
+ return true;
+ return high != (b < 0
+ ? ~(unsigned HOST_WIDE_INT) 0
+ : 0);
+}
+
# define OMP_CLAUSE_MASK_1 omp_clause_mask (1)
#endif
--- gcc/c-family/c-omp.c.jj 2013-10-11 11:23:59.000000000 +0200
+++ gcc/c-family/c-omp.c 2013-10-11 15:22:36.506375807 +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;
