User-facing threadmode query
We document how a user can direct a compilation to use the SEQ or PAR backends.
However, we identified today a lack of a corresponding way for user code to identify which backend is being used.
There are multiple parts to this:
1. Design
We need to pick an interface for this.
A preprocessor symbol is one simple mechanism, but the UPCXX_BACKEND_GASNET_{SEQ,PAR}
used internally are NOT good names to dump on a user. Additionally, this isn't cleanly extensible (not that we actually anticipate more backends).
There are probably C++ish ways to accomplish this in the upcxx::
namespace as well.
2. Implementing
3. Documentation
Comments (10)
-
reporter -
I'm worried the named constants from proposal 1 might encourage mistakes from naive users such as :
#if UPCXX_THREADMODE_PAR // do some lock thing #endif
this looks plausibly correct but does not function as intended.
A similar mistake:
#ifdef UPCXX_THREADMODE // do some lock thing #endif
also looks plausibly correct but does not function as intended.
I think my preference would be
UPCXX_THREADMODE
which is documented as undef for SEQ mode and defined to non-zero for PAR mode. This seems to have the least potential for accidental mis-use, while still allowing extension to possible future backends.Of course we could additionally supply a
constexpr upcxx::thread_mode
enum with appropriate values, providing stronger type-checking for codes that don't need preprocessor exclusion, but that might be overkill.One downside to all these plans is users writing codes to work in 2020.3.0 and earlier will probably still need to inspect
UPCXX_BACKEND_GASNET_{SEQ,PAR}
, until this new scheme is deployed and permeates installs in the field. -
reporter Dan's concerns immediately above both make sense.
I will plan to PR (as time allows) his proposal:UPCXX_THREADMODE which is documented as undef for SEQ mode and defined to non-zero for PAR mode
The implementation is probably no more than
#undef UPCXX_THREADMODE // paranoia #if UPCXX_BACKEND_GASNET_PAR #define UPCXX_THREADMODE 1 #endif
-
reporter -
assigned issue to
-
assigned issue to
-
reporter - changed status to open
-
reporter Proposed implementation in pull request 203.
Proposed documentation in Prog Guide pull request 15 -
I think we should consider expanding this work to include a
UPCXX_CODEMODE
symbol, for symmetry and to provide users a symbol matching the outward-facing name, instead of having them rely upon the undocumented internalUPCXX_ASSERT_ENABLED
.Example usage:
#include <upcxx/upcxx.hpp> #if UPCXX_CODEMODE // nonzero == production #define NDEBUG 1 // disable assert() in my program #endif #include <assert.h>
-
reporter Agree. Will add UPCXX_CODEMODE (impl and docs) to existing PRs.
-
reporter Implement UPCXX_THREADMODE
This commit provides an implementation of
UPCXX_THREADMODE
, a preprocessor identifier defined non-zero for PAR and undefined for SEQ. The non-zero value is intentionally obfuscated.This commit partially addresses issue 325.
→ <<cset 7e880a8c5928>>
-
reporter - changed status to resolved
With merge of both the impl and guide PRs, this issue is resolved.
- Log in to comment
I had originally stated "not that we actually anticipate more backends", but John's recent text regarding Open MP acknowledged that might actually be a possibility some day. So, we should definitely keep that in mind. However, an enum that expands to include additional multi-threaded modes, not known in programs written before, could result in incorrect behavior if not used properly. So, at least at this point in time I believe we are looking at exposing a single boolean property: either "is SEQ" or "is PAR". If the "PAR" portion of that space of that should ever incorporate multiple distinct models, then we can name them later.
Proposal 1:
I don't have any other proposals now, as I believe the ability to use the preprocessor is critical.
However, If named this "proposal 1" to avoid confusion should there be additional ones before this is settled.
Thoughts?