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

[GoogleCodeExporter]

This addresses items #3 and #30 from the UPC Language Specification Issues list:

#3: should MYTHREAD and THREADS be specified as size_t to agree with return 
type of upc_threadof()? (not backwards compatible)

#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:

#3 proposes going with size_t
#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.

Original issue reported on code.google.com by phhargr...@lbl.gov on 22 May 2012 at 2:17

[GoogleCodeExporter]

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.

Original comment by brian.wibecan on 22 May 2012 at 9:51

[GoogleCodeExporter]

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.

Original comment by danbonachea on 31 May 2012 at 6:17

[GoogleCodeExporter]

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

Original comment by phhargr...@lbl.gov on 31 May 2012 at 6:01

[GoogleCodeExporter]

Original comment by phhargr...@lbl.gov on 1 Jun 2012 at 3:20

• Added labels: Spec-1.3, Spec-2.0

[GoogleCodeExporter]

Original comment by phhargr...@lbl.gov on 1 Jun 2012 at 6:09

• Added labels: Milestone-Spec-1.3
• Removed labels: Spec-1.3, Spec-2.0

[GoogleCodeExporter]

Update summary to match nature of current proposal

Original comment by phhargr...@lbl.gov on 14 Jun 2012 at 11:09

• Changed title: modification: THREADS/MYTHREAD have "(unsigned?) integral value" rather than "type int"

[GoogleCodeExporter]

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.

Original comment by danbonachea on 15 Jun 2012 at 7:37

[GoogleCodeExporter]

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

Original comment by phhargr...@lbl.gov on 15 Jun 2012 at 7:43

[GoogleCodeExporter]

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. 

Original comment by danbonachea on 16 Aug 2012 at 10:55

[GoogleCodeExporter]

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

Original comment by danbonachea on 17 Aug 2012 at 3:25

• Changed state: PendingApproval

[GoogleCodeExporter]

Set Consensus:High on Pending Approval issues.

Original comment by gary.funck on 19 Aug 2012 at 11:31

• Added labels: Consensus-High

[GoogleCodeExporter]

Comment #9 applied in SVN r111

Original comment by danbonachea on 14 Sep 2012 at 12:48

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

[GoogleCodeExporter]

Original comment by danbonachea on 14 Sep 2012 at 12:56

• Changed state: Fixed

[GoogleCodeExporter]

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.

Original comment by danbonachea on 14 Sep 2012 at 7:26

[GoogleCodeExporter]

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

Original comment by danbonachea on 14 Sep 2012 at 9:46

• Changed state: Ratified