Remove "Short" and "unsigned Short" from the supported data types for Shmem_wait/Shmem_test APIs

[rdesai16]

I came across this issue while implementing vector APIs for shmem_test and shmem_wait. As of a recent change from PR #267 , shmem_wait and shmem_test only work with AMOs and based on this, the list of datatypes supported by both AMOs and wait/test APIs, should be the same. But currently that is not the case.

So basically the issue is that in current version, AMOs do not support data types "short" and "unsigned short" whereas wait/test APIs do. Hence, shouldn't we remove "short" and "unsigned short" from the supported data types list of wait/test APIs?

Any thoughts?

[jdinan]

👍 I would suggest going a bit further and updating the wait/test routines to be defined according to the types in the extended AMOs table, removing the sync types table, and deprecating routines for the types not in the extended AMOs table. Will need to double check that the signal type is in the extended AMOs table.

[davidozog]

I agree that we definitely want to consider removing the point-to-point sync types table and replacing with the appropriate AMO table. I would guess that we wouldn't want the floating point types in the extended AMOs table to be compatible with the p2p sync types though (due to FP equality issues...). Are you thinking we support comparisons like CMP_EQ on FP types @jdinan?

And yeah, the signal type is only uint64 which is in both the standard and extended AMO tables.

[jdinan]

Yikes, good point about the floating point types.

[naveen-rn]

Will need to double check that the signal type is in the extended AMOs table.

Please clarify.

[davidozog]

Will need to double check that the signal type is in the extended AMOs table.

Please clarify.

I think @jdinan meant "standard" (not extended) AMOs table, and we need to assure the "signal type" (uint64_t) is covered by the proposed change - and it is. I've posted that change here.

[naveen-rn]

Fixed in https://github.com/naveen-rn/specification/pull/3

[naveen-rn]

Can we close this issue? As the change is made in the above mentioned PR? @rdesai16

[jdinan]

The fix looks good. Did it also get a changelog entry?

[rdesai16]

@naveen-rn Yes. The fix looks good and is merged so we can close this issue. We should just make sure of the changelog entry.

[jdinan]

Whoops, looks like we still need the changelog entry. Reopening.

[nspark]

@BryantLam Do you want to take this one? It seems you've had to resolve a lot of conflicts in the changelog already.

[BryantLam]

Can do.