Add is_shared(T*) query

[Dan Bonachea]

Problem Statement:

UPC++ currently has the following semantic abilities wrt global_ptr and the shared heap:

1. global_ptr<T>::local() - downcast operation that converts a gptr to a raw ptr
2. global_ptr<T>::is_local() - query that tells you when downcasting is permitted for a gptr
3. global_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.

[Paul Hargrove]

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"?

[Amir Kamil]

Merged in d57cd44.

[john bachan]

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 eliminate is_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
}

[Dan Bonachea]

I believe John's proposed try_upcast is strictly stronger than the specified is_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
}

[john bachan]

I'm not in love with tag arguments, adds an extra upcxx:: qualifier. @akamil preference?

[Amir Kamil]

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).

[Dan Bonachea]

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).

[john bachan]

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 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. 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.

[Dan Bonachea]

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.

[Amir Kamil]

std::nothrow is used as a tag in the standard library. I'm not sure it's used anywhere for a constructor though.

[john bachan]

I would be in favor of either of Dan's sets of free functions, though I lean towards the first (separate "to" and "try").

[Dan Bonachea]

Resolved in 2d9680f (pull request #16)