Add support for put with signal operation

[naveen-rn]

This is a work-in-progress proposal to add put with signal operations into the OpenSHMEM standards.
This is related to issue #206. Proposed feature is available as part of Cray SHMEM implementation.

[naveen-rn]

Some PENDING ITEMS that needs further discussions:

• add nbi support - can we add as a single proposal?
  opening a separate PR to add this feature.
• for zero size puts - send only the signal is the expected behavior - does Annex C already capture semantics?
  yes, this is captured and we don't have any special text in this proposal for specifcally handling put-with-signal
• do we need explanation about consecutive puts_signals or put with another put_signal?
  we have added a generic explanation

[agrippa]

This is probably a question that has been asked and answered in other contexts but: are there any atomicity guarantees on the delivery of the signal? Should there be a note here calling that out? If there are no guarantees, should/could there be a separate set of APIs that deliver the signal atomically (possibly as an atomic operation like AND and not just an atomic set)?

[nspark]

I think the answers to your questions lie with the resolution of #204, #229, and the overall memory model work. I think one goal of those efforts should be that shmem_wait_until (and family) can be safely triggered by atomic operations and the signal of a put-with-signal. If that requires the signal to be an atomic remote write, then so be it.

[agrippa]

To your knowledge, has there been investigation into something like put-with-atomic-op? Semantically a putmem followed by an atomic operation (e.g. AND).

Is there any benefit in hardware to doing something like that, or would implementations just support it as putmem-fence-atomic?

[nspark]

The closest I can think of is the "counting puts" proposal from @jdinan & Co. This is equivalent to a put+fence+atomic-add. That is certainly a useful idiom. I'm not sure whether the full generality of put+fence+atomic-op is useful for all operations (i.e., set, add, and, or, xor).

I definitely think there would be application benefit in having hardware support for these idioms. It is often the case that fence (ordering, not necessarily completion) requires completion, due to network API constraints. If that fence can be avoided because the interconnect manages the ordering and atomic op, then the user is introducing fewer latency-sensitive operations.

[shamisp]

@nspark ordered network will help but if you take in account something like adaptive routing it may break it. You really want to have the packet small enough such that you can squeeze it in single MTU.

…

On Tue, Jul 24, 2018 at 11:11 AM Nick Park ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In content/shmem_put_signal.tex <#218 (comment)> : > +\apireturnvalues{ + None. +} + +\apinotes{ + The \VAR{dest} and \VAR{sig\_addr} data object must both be remotely + accessible, but may each be allocated from the symmetric heap or global/ + static memory. + + The delivery of \signal{} flag on the remote \ac{PE} indicates only the + delivery of its corresponding \dest{} data words into the data object on + the remote \ac{PE}. Without a memory-ordering operation, there is no implied + ordering between the delivery of the signal word of a put-with-signal + routine and another data transfer. For example, the delivery of the signal + word in a sequence consisting of a put routine followed by a put-with-signal + routine does not imply delivery of the put routine's data. The closest I can think of is the "counting puts" proposal from @jdinan <https://github.com/jdinan> & Co. This is equivalent to a put+fence+atomic-add. That is certainly a useful idiom. I'm not sure whether the full generality of put+fence+atomic-op is useful for all operations (i.e., set, add, and, or, xor). I definitely think there would be application benefit in having hardware support for these idioms. It is often the case that fence (ordering, not necessarily completion) requires completion, due to network API constraints. If that fence can be avoided because the interconnect manages the ordering and atomic op, then the user is introducing fewer latency-sensitive operations. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#218 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACIe2MKlfsy53iL2jt63b6zxKk1JCJEjks5uJ0cYgaJpZM4UOpmX> .

[nspark]

August F2F discussion:

• @minsii: Should we add clarifications suggesting shmem_wait_until or shmem_test to clarify portable practices for accessing the signal word?
• @anshumang: Do we explicitly prohibit the signal word from overlapping with the source or destination buffers?
  • Edit: @nspark suggests prohibiting this overlap via restrict.

[manjugv]

August F2F : Ballot postponed.

[naveen-rn]

@minsii and @shamisp, I suppose a simple reference to put-with-signal (as in 76a5015) in the list of shmem routines which enforces ordering through p2p_sync operations would clarify the portable practice for using put-with-signal. Let me know, If we need some more clarifications.

[naveen-rn]

@nspark @anshumang As modified in 94976fe, I suppose we are planning to restrict the sig_addr buffer to be distinct from source and dest buffer. sig_addr and dest/source must not be same or overlapping data objects.

[naveen-rn]

Based on discussions with @anshumang and others -
@nspark It doesn't look like we could get some form of compiler optimizations from the restrict qualifier and also the primary purpose of restrict is to guide users. So, I'm not sure whether we need the restrict qualifier in the API - can just the details in the description section be sufficient?

[naveen-rn]

To update, based on RMA WG discussions, we are planning to drop restrict qualifier for put-with-signal and can add it when we go through the whole specification. This is because of the following reasons:

1. Adding restrict to signal buffer makes sense - but not to both destination and source buffer - that makes it too restrictive
2. Avoid unnecessary compilation warnings

[minsii]

@naveen-rn Thanks for the clarification fix with point-to-point sync. I have one more question: why do you define sig_addr with uint64_t type ?

[naveen-rn]

@minsii In general, I would prefer a fixed width data type for representing the size of the signal, rather than using generic types which varies with platform. If I'm correct the C standards limits the width to 64. Also, the current implementation in Cray hardware supports only 64 bits for signal - hence we matched that with uint64_t. It can be int64_t - if we prefer the signal to be negative. Are you interested in something different? Say an opaque object like - shmem_sig_t and restrict the size of this opaque object to 64 bits?

[naveen-rn]

Changes made after the October meeting:
https://github.com/openshmem-org/specification/compare/fd54dd3..ada81c9
The following changes were made:

1. removed restrict qualifier
2. updated the description section to contain the same explanation from summary
3. updated context argument explanation
4. added better explanation for sig_addr and dest overlap
5. avoided using shmem_malloc and shifted to using shmem_calloc in example
6. changed the size parameter input in example
7. modified all type-based shmem routines to C11-generic routines in the example
8. removed print option from example
9. clarified that the signal update is an atomic operation

We could use these changes for Jan 2019 OpenSHMEM specification meeting.
[EDIT]: Updated after rebasing with master

[manjugv]

January 2019 Meeting: Voting postponed

[shamisp]

I would suggest to clarify what "exclusive" means in this context.

[manjugv]

@naveen-rn @jdinan As I mentioned in the WG meeting, I think it would be useful for defining partial and complete updates. I created an issue to track this and referenced both tickets that gets impacted.

[jdinan]

@naveen-rn Can this one also be closed?

[naveen-rn]

Closing this PR as we have opened a new composed PR with both blocking and non-blocking put-with-signal: #275