Add is_shared(T*) query
Problem Statement:
UPC++ currently has the following semantic abilities wrt global_ptr and the shared heap:
global_ptr<T>::local()
- downcast operation that converts a gptr to a raw ptrglobal_ptr<T>::is_local()
- query that tells you when downcasting is permitted for a gptrglobal_ptr<T>:: global_ptr (T*)
- upcast operation that converts a raw ptr to a gptr
Operation 3 has a precondition:
ptr must be either null or an address in the shared segment of a rank in the local team
However there is currently no way to express this precondition as a query within the semantics of the system. I believe this is a semantic gap that should be closed.
Proposed Solution:
Add the following query:
==========================================
bool upcxx::is_shared<T>(T *ptr);
Precondition: ptr
is a valid pointer such that the expression *ptr
on the calling rank yields an object of type T.
Returns true if and only if the object referenced by *ptr
resides within the shared segment (Ch. 4) of a rank in the local team (Sec 12.2).
==========================================
Note this is written to dodge the question of memory kinds - ie this is only for the main memory "kind". It also prohibits nullptr inputs, although that decision can be changed without affecting the utility. It also prohibits T = "pointer to function" type, but that's fine because functions in the code segment are never shared anyhow, so the answer is already statically known without the query call.
We could potentially make this a static member function of global_ptr<T> if people prefer that design.
Analysis
With this in place, the precondition for upcast can be simplified to something like:
ptr == nullptr || is_shared(ptr)
More importantly, user code gains the ability to query whether a raw ptr input with unknown sharing properties is valid to upcast. This could be a useful modularity feature - for example an operation that wants to send a global pointer to some local data to a remote rank for later rget could query whether it can directly upcast its own input, or whether it needs to allocate a shared temporary and copy the data.
Comments (12)
-
-
- changed status to resolved
Merged in d57cd44.
-
Wouldn't it make sense to merge the (likely) combo of
is_shared
followed by upcast into a single nicely failing upcast? Then we could eliminateis_shared
. Upcast requires binary search over segment address ranges, would be nice to do this only once.Maybe:
static global_ptr<T> global_ptr::try_upcast(T*); // returns null on fail or on null input // user code if( (global_ptr<T> gp = global_ptr<T>::try_upcast(local_ptr)) ) { // gp is non-null, upcast succeeded } else { // gp is null, not shared memory }
-
reporter - changed status to open
I believe John's proposed
try_upcast
is strictly stronger than the specifiedis_shared
query, so I believe it meets the semantic need. Most of my envisioned clients for this operation would be following successful return with an upcast, so it makes sense.Regarding naming, it's a bit asymmetrical that the existing upcast (with in-segment precondition) is a constructor, whereas John's proposed
try_upcast
is a factory function. Would it make more sense for this to be a separate constructor, disambiguated with a tag argument? This would provide the same syntactic convenience of the regular constructors, eg:global_ptr<T> gp(local_ptr, upcxx::maybe_shared); if (gp) { // upcast successful } else { // not in shared mem }
-
I'm not in love with tag arguments, adds an extra
upcxx::
qualifier. @akamil preference? -
No strong preference, but if we go with a tag argument, I'd prefer something that can be reused in other contexts (e.g.
upcxx::check
or something like that). -
reporter I'm not in love with tag arguments, adds an extra upcxx:: qualifier.
Assuming the user imports nothing, I believe both of the examples above have the same number of
upcxx::
qualifiers, because the constructor version avoids the need to spell out the fully-qualified global_ptr typename a second time.My general feeling on this is we should do our best to adhere to C++ standard library design patterns except where there is sound technical motivation for deviation (for example name collision or concepts that don't generalize to an asynchronous PGAS environment), even when it doesn't match our personal stylistic preferences. Our user base is predicated on familiarity with the modern C++ library, so designing our extension library API in the same style as STL lowers the learning curve. Tagged constructors seem to be a common pattern in the STL.
That being said, another potential solution here is to add a defaulted argument to the existing upcast constructor:
template <typename T> global_ptr <T>::global_ptr(T* ptr, bool null_on_failure = false); // Precondition: if null_on_failure is false, then ptr must be either null // or an address in the shared segment (Ch. 4) of a process in the local team (§11.2) of the caller
where the default value of false selects the current behavior, and true activates the conditional upcast behavior that returns a null gptr on failure.
With this API, the running example becomes even more concise:
global_ptr<T> gp(local_ptr, true /*conditional upcast*/); if (gp) { // upcast successful } else { // not in shared mem }
and the optimized code generation should look and perform similarly under the assumption the new second argument is a manifest constant expression (which it would be in all the important use cases I can think of).
-
Speaking from my personal experience, tagged constructors are not used often, as in I can't remember using a tagged constructor supplied by
std
or anyone else's code.I also think boolean arguments are usually a bad idea because the call site rarely makes it obvious what meaning
true
/false
are encoding.If we consider local/global casting as a conversion, then looking at
std
we see thatstring
does not provide constructor overloads for converting various data types to string form. They havestd::to_string<T=int/float/etc.>(T)
.My preference is to decouple interesting logic from construction. Constructors should be few (since they are not an extensible set for anybody but the owner of that type's header file), while factory-like free functions are extensible and can be composed over to make ever more interesting factory logic.
-
reporter If we consider local/global casting as a conversion, then looking at std we see that string does not provide constructor overloads for converting various data types to string form. They have std::to_string<T=int/float/etc.>(T). ... My preference is to decouple interesting logic from construction.
By this logic, we should be providing both forms of upcast as factories in the
upcxx
namespace:global_ptr<T> upcxx::to_global_ptr<T>(T *local_ptr); // normal upcast. precond: in shared seg global_ptr<T> upcxx::try_global_ptr<T>(T *local_ptr); // conditional upcast
Is this what you are advocating? I think in both cases upcast constitutes "interesting logic" that is more than just simple object initialization.
Alternatively, we could just specify that
upcxx::to_global_ptr<T>(T *)
always performs conditional upcast, which makes it suitable for use in any upcast situation. -
std::nothrow
is used as a tag in the standard library. I'm not sure it's used anywhere for a constructor though. -
I would be in favor of either of Dan's sets of free functions, though I lean towards the first (separate "to" and "try").
-
reporter - changed status to resolved
Resolved in 2d9680f (pull request #16)
- Log in to comment
I wanted to ask what we think the behavior should be for "Memory Kinds". Specifically: is a
T*
referencing GPU memory mapped into the host's virtual address space "shared"?