warnings from GCC 10.1 in reduce.hpp for boolean reductions
test/collectives.cpp is triggering a warning from GCC 10.1 in reduce.hpp for boolean reductions on at least one platform ( EX-ppc64el-smp-gcc-cupc2c, which simulates GCC on Summit).
The meat of the warning:
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/bin/upcxx -o collectives-par /home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx/test/collectives.cpp -Wall -Wno-unused
In file included from /home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx/test/collectives.cpp:4:
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp: In instantiation of 'T upcxx::detail::opfn_mul::operator()(T, Tb&&) const [with T = bool; Tb = bool&]':
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp:120:25: required from 'static void upcxx::detail::reduce_op_slow_id<Op, T>::op_vecfn(const void*, void*, std::size_t, const void*) [with Op = upcxx::detail::op_wrap<upcxx::detail::opfn_mul, true>; T = bool; std::size_t = long unsigned int]'
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp:294:50: required from 'typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::reduce_scalar_event_values<T>, Cxs>::return_t upcxx::detail::reduce_one_or_all_trivial(T1&&, BinaryOp, upcxx::intrank_t, const upcxx::team&, Cxs) [with T1 = bool; BinaryOp = upcxx::detail::op_wrap<upcxx::detail::opfn_mul, true>; Cxs = upcxx::completions<upcxx::future_cx<upcxx::operation_cx_event, upcxx::progress_level::user> >; T = bool; typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::reduce_scalar_event_values<T>, Cxs>::return_t = upcxx::future1<upcxx::detail::future_kind_shref<upcxx::detail::future_header_ops_general>, bool>; upcxx::intrank_t = int]'
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp:518:64: required from 'typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::reduce_scalar_event_values<T>, Cxs>::return_t upcxx::reduce_all(T1, BinaryOp, const upcxx::team&, Cxs) [with T1 = bool; BinaryOp = upcxx::detail::op_wrap<upcxx::detail::opfn_mul, true>; Cxs = upcxx::completions<upcxx::future_cx<upcxx::operation_cx_event, upcxx::progress_level::user> >; T = bool; typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::reduce_scalar_event_values<T>, Cxs>::return_t = upcxx::future1<upcxx::detail::future_kind_shref<upcxx::detail::future_header_ops_general>, bool>]'
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx/test/collectives.cpp:88:76: required from here
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp:49:18: warning: '*' in boolean context, suggest '&&' instead [-Wint-in-bool-context]
40 | a tok##= std::forward<Tb>(b);\
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......
49 | UPCXX_INFIX_OP(*, mul, false)
/home/phargrov/upcnightly/EX-ppc64el-smp-gcc-cupc2c/runtime/work/dbg/upcxx-inst/upcxx.debug.gasnet_par.smp/include/upcxx/reduce.hpp:40:13: note: in definition of macro 'UPCXX_INFIX_OP'
40 | a tok##= std::forward<Tb>(b);\
| ^~~
Basically -Wint-in-bool-context
(enabled by -Wall
) wants us to explicitly use &&
for and-ing bool
types, rather than (confusingly) relying on *
to have that same effect via integral promotion and [conv.bool]. Of course this arises here because we are using macro/template expansion that applies to all integral types.
I believe the warning is harmless in this particular case, but would rather not globally suppress the warning (especially because this occurs in a public header, so that would impact user code). Ideally we can find a localized workaround in the header, either by special-casing bool
or adding scoped warning suppression.
Comments (3)
-
-
reporter We are purposely testing bool with op_fast_mul. I don’t think we have a right to expect that to work without a warning.
The spec explicitly permits this usage in 12.2-22:
If T is bool, then op_fast_add and op_fast_max perform the same operation as op_fast_bit_or, and op_fast_mul and op_fast_min perform the same op- eration as op_fast_bit_and.
Whether it should generate a warning or not is debatable, but I don't like to see warnings in library code for behaviors we explicitly permit.
Your fix looks right to me.
-
- changed status to resolved
Add bool overloads for reduction operators. Fixes
#376.→ <<cset 35bb0ac3cb9e>>
- Log in to comment
This is fixable with an operator overload for
bool
:However, the real culprit is the test code in
collectives.cpp
:We are purposely testing
bool
withop_fast_mul
. I don’t think we have a right to expect that to work without a warning.