modification: THREADS/MYTHREAD have "integral value" rather than "type int"

Issue #32 new
Former user created an issue

Originally reported on Google Code with ID 32 ``` This addresses items #3 and #30 from the UPC Language Specification Issues list:

  1. 3: should MYTHREAD and THREADS be specified as size_t to agree with return type of upc_threadof()? (not backwards compatible)
  1. 30: Propose a upc_thread_t type (unsigned, integral) to use for MYTHREAD, THREADS, upc_threadof.

There are currently two types used for a UPC thread number:

1) The predefined identifiers THREADS and MYTHREAD are defined in section 6.3 as having type "int".

2) The function upc_threadof() is defined in section 7.2.3.1 as having type "size_t".

Finding a single common type for these would address: + Some (many) compilers will warn about operations (mostly comparisons, and arithmetic, but sometimes assignment) which mix signed and unsigned types + Some (few?) compilers warn about implicit narrowing conversions + Just makes more sense!

Some (unordered) observations: + On most platforms an "int" is large enough for 2 billion UPC threads + On many platforms size_t is big enough for 18,446,744,073,709,551,615 (18 quintillion) threads + There is no obvious necessity for a SIGNED type for a thread number, since the value THREADS is always available as an invalid value where such is needed.

The items #3 and #30 propose two means by which to resolve this difference:

  1. 3 proposes going with size_t
  2. 30 proposes introducing a new unsigned integral upc_thread_t type

One thing they both agree upon is the use of an unsigned type. As noted above, there is no compiling reason I can come up with to require representing negative thread numbers. So, I will assume that if we change anything, it will be to a UNsigned type. The observation that some compilers may warn about operations mixing signed-vs-unsigned types has no "perfect" resolution since we are starting with one of each.

As noted above, an integer should be "wide enough" for 2-billion threads, and a change to unsigned integer would be enough for 4-billion.

As I've noted in my comment on at least one of other issue, changing the width of anything has the potential to make "bad things" happen where printf() or scanf() are currently using the correct modifiers. Therefore I would caution against making any change lightly. In addition, of course, many codes that would continue w/o introducing errors would experience new warnings.

The feeling I have is: + It is better to use an unsigned type + Though use of size_t looks excessive today, 4-billion UPC threads is not as many as you may think. + The loss of range by having a SIGNED value for THREADS and MYTHREAD is not, today, going to prevent anybody's code from running + the warnings that may result today from if (MYTHREAD == upc_threadof(p)) can be suppressed with a cast. + All told the current situation is less harmful than the potential impact of changing the type(s).

So, my proposal is:

1) DO NOT make any change yet for UPC 1.3.

2) For UPC 2.0 we should use an implementation-dependent unsigned integral type for "upc_thread_t", which would become the type for both the predefined identifiers THREADS and MYTHREAD, and for the upc_threadof() function (or operator?). The implementation would also have to supply a corresponding UPC_THREAD_MAX value. This would allow system-appropriate types to be chosen. For instance, very few compilers would be expected to use a 64-bit type in 2012, but in 2020 it might be the common case. ```

Reported by `phhargrove@lbl.gov` on 2012-05-22 02:17:14

Comments (15)

  1. Former user Account Deleted

    ``` I agree with the scheduling of the proposal.

    I agree that MYTHREAD/upc_threadof is the main issue, but I would also like to see upc_addr_t (upc_addrfield_t? something like that) and upc_phase_t (upc_blocksize_t? something like that) introduced. upc_blocksizeof and upc_phaseof can return the same type. Only upc_blocksizeof returns something that can be called a size.

    Side note, I really dislike rampant use of casting. It can cover a multitude of problems. ```

    Reported by `brian.wibecan` on 2012-05-22 21:51:43

  2. Former user Account Deleted

    ``` As noted in issue 33, MYTHREAD and THREADS are currently specified as "EXPRESSIONS with a value of type int". This should not be confused with "objects with a value of type int". This was an deliberate decision intended to prevent exactly the sort of potential pitfalls described above.

    By ISO-C 6.5.16.1: "In simple assignment (=), the value of the right operand is converted to the type of the assignment expression and replaces the value stored in the object designated by the left operand."

    This implies that MYTHREAD and THREADS should already be assignment compatible with any integral type on the LHS, and no warnings should occur. Truncation occurs during assignment, so whether or not an overflow occurs is determined by whether the object type of the LHS provided by the user was sufficiently wide to hold the value.

    If anything I believe the current spec says too much about the expression type - it should be sufficient to relax the phrase "of type int" to "of integral type". This would leave the exact type up to the implementation, which can choose an appropriately wide expression type.

    I think there's a separate issue here of whether upc.h should provide an integral typedef for a type which is guaranteed to be sufficiently wide to represent THREADS for the current translation environment. This would be as a convenience to users who wish to portably store thread ids in their data structures, but is not directly related to the type of THREADS and MYTHREAD.

    On the issue of upc_threadof(), this is a library function and as such required specifying a declared return type, for the purposes of type compatibility checking, etc. size_t was convenient and guaranteed to be sufficiently wide, because the expression sizeof(shared [1] char [THREADS]) is specified to have type size_t. By this logic size_t can also currently be used to serve as the convenience type mentioned above.

    If we decide to introduce a convenience type for thread ids it would make sense to amend the return type.

    ```

    Reported by `danbonachea` on 2012-05-31 06:17:31

  3. Former user Account Deleted

    ``` As owner for this issue, and based on the comments made so far, I am proposing the following "resolution".

    Proposed for UPC 1.3: Replace, as Dan suggests, the phrase "with a value of type int" with EITHER "of intergral type" OR "of unsigned intergral type" This would *not*, as far as I can determine, introduce backward compatibility problems (correct me if I am wrong) if current implementation don't actually change their types. It "opens the door" for such a change later. We do still need to agree on the signed/unsigned issue (my vote is for UNsigned). The unsigned choice would, at least, resolve the warnings which some compilers produce when mixing signed and unsigned types.

    Proposed for UPC 2.0: Define a "upc_thread_t" to replace the return type of upc_threadof() This is "unsafe" (and thus not for 1.x) because either narrowing or widening of any type is not backwards-compatible (think "%z" in a printf).

    ```

    Reported by `phhargrove@lbl.gov` on 2012-05-31 18:01:40

  4. Former user Account Deleted

    Reported by `phhargrove@lbl.gov` on 2012-06-01 03:20:16 - Labels added: Spec-1.3, Spec-2.0

  5. Former user Account Deleted

    Reported by `phhargrove@lbl.gov` on 2012-06-01 06:09:11 - Labels added: Milestone-Spec-1.3 - Labels removed: Spec-1.3, Spec-2.0

  6. Former user Account Deleted

    ``` Update summary to match nature of current proposal ```

    Reported by `phhargrove@lbl.gov` on 2012-06-14 23:09:59

  7. Former user Account Deleted

    ```

    The idea of this relaxation is to let the implementation entirely determine an appropriate expression type, since the semantics of assignment already ensure implicit conversion that prohibit any compiler warnings and prevent the user from even observing what type is in use for that expression by the compiler.

    Upon further digging, C99 uses the abstract term "integral" to refer to a value which is mathematically an integer (ie a value with no fractional component). It is not used to refer to the language type system, and the terms "signed", "unsigned" and "type" do not appear anywhere with "integral".

    Amended proposal for 1.3 spec:

    "MYTHREAD/THREADS is an expression with integral value;"

    The subsequent sentence in each paragraph already make it clear that THREADS > 0 and MYTHREAD >= 0. ```

    Reported by `danbonachea` on 2012-06-15 07:37:38

  8. Former user Account Deleted

    ``` I endorse Dan's amended proposal appearing in comment #7. ```

    Reported by `phhargrove@lbl.gov` on 2012-06-15 07:43:10

  9. Former user Account Deleted

    ```

    Proposed Change (omitting changebar markup): -------------------------------------------------------------------- --- upc-language.tex (revision 80) +++ upc-language.tex (working copy) @@ -71,15 +71,18 @@ \subsubsection\tt THREADS \label{threads} \index{THREADS} -\npf{\tt THREADS} is an expression with a value of type int; it specifies the number of +\npf{\tt THREADS} is an expression with integral value; it specifies the number of threads and has the same value on every thread. Under the static THREADS translation environment, {\tt THREADS} is an integer constant suitable for use in {\tt \#if} preprocessing directives.

    \subsubsection\tt MYTHREAD \index{MYTHREAD} -\npf{\tt MYTHREAD} is an expression with a value of type int; it specifies the +\npf{\tt MYTHREAD} is an expression with integral value; it specifies the unique thread index.

    ```

    Reported by `danbonachea` on 2012-08-16 22:55:51

  10. Former user Account Deleted

    ``` Mass-edit to mark all issues with an officially-announced resolution diff as "PendingApproval" ```

    Reported by `danbonachea` on 2012-08-17 15:25:01 - Status changed: `PendingApproval`

  11. Former user Account Deleted

    ``` Set Consensus:High on Pending Approval issues. ```

    Reported by `gary.funck` on 2012-08-19 23:31:54 - Labels added: Consensus-High

  12. Former user Account Deleted

    ``` For the record: Public comment period for this change was 8/14/2012 - 9/14/2012 No substantial objections were raised or recorded during that period. At the 9/7/2012 telecon, it was announced this change was imminent, feedback was explicitly solicited, and none was received. The change was integrated into a working draft distributed on 9/13/2012 for consideration and draft ratification at the 9/14/2012 telecon. ```

    Reported by `danbonachea` on 2012-09-14 07:26:14

  13. Former user Account Deleted

    ``` Changes distributed as Draft 1.1 were ratified during the 9/14/2012 teleconference, for inclusion in the next public draft. ```

    Reported by `danbonachea` on 2012-09-14 21:46:01 - Status changed: `Ratified`

  14. Log in to comment