Implement upc_threadof, upc_phaseof, and upc_addrfield as operators

[GoogleCodeExporter]

This proposal is responsive to open issues 41 and 50.

41. Are there advantages to defining certain library functions such as 
upc_threadof, upc_phaseof, and upc_addrfield as unary operators rather than 
functions/macros defined in the core library?

50. Re: advantages of defining upc_threadof and related functions as operators, 
an operator is capable of adapting to a wider variety of argument types. For 
example, this program results in a warning ("‘upc_threadof’ discards 
‘strict’ qualifier from pointer target type") when compiled with GCC/UPC:

  strict shared int a;
  thread = upc_threadof (&a);

Although this warning may not be mandated by the standard, "C" compilers will 
typically warn about this sort of usage, and there is no obvious work-around 
because the function argument type matching rules will dictate the nature of 
this check.

This proposal takes the position that upc_threadof, upc_phaseof, and 
upc_addrfield should be defined as operators returning a value of type 
'size_t'.  The case might be made that upc_addrfield might return some other 
type such as 'uintptr_t' (defined in stdint.h) but this would not be backwards 
compatible, strictly speaking with the existing definition.  The motivation for 
choosing another type name is that 'size_t' is defined as the type used to 
return values from the sizeof operator.

The reasons in favor of defining these UPC pointer-to-shared value manipulation 
operators is that they can: (1) likely be more efficiently implemented and (2) 
can adapt to the type of value passed to them without being subject to function 
argument type matching rules, thus avoiding the situations where spurious 
warnings might be generated as described in language issue 50 (cited above).

An alternative to solving the function argument mismatch use case described 
above is to relax function argument type compatibility checking with respect to 
pointers-to-shared that are either 'strict' or 'relaxed' qualified when the 
target type is (shared void *).  This alternative will be described in a 
separate proposal.  Note however, that both this proposal (defining 
upc_threadof, upc_phaseof, and upc_addrfield as operators) can be reviewed 
and/or accepted separately from the alternative proposal of relaxing type 
qualifier compatibility for generic pointers-to-shared.

Original issue reported on code.google.com by gary.funck on 21 May 2012 at 4:36

• Blocking: #20

[GoogleCodeExporter]

If efficiency were the main motivation for accepting this proposal, it should 
be noted that the fact that library functions can also be implemented as macros 
(see issue #18) might mitigate performance concerns.

For example, upc_threadof() might be defined as follows:

#define upc_threadof(p) __builtin_threadof (p)

where __builtin_threadof() is a compiler-implemented builtin function.
Alternatively, the macro might be UPC runtime-specific, invoking an access 
function defined either as a macro or an inlined function in order to provide 
efficient access to the various fields of a pointer-to-shared value.

Original comment by gary.funck on 21 May 2012 at 4:43

[GoogleCodeExporter]

I don't currently see the "need" for changing these functions to be operators.  
Extending Gary's previous comment slightly, one can address the type 
compatibility issue by "casting away" the qualifiers:

#define upc_threadof(p) __builtin_threadof((shared void *)p)

If I am missing something, please let me know.

Original comment by phhargr...@lbl.gov on 22 May 2012 at 12:02

[GoogleCodeExporter]

> I don't currently see the "need" for changing these functions to be operators.
> Extending Gary's previous comment slightly, one can address the type 
compatibility
> issue by "casting away" the qualifiers:

#define upc_threadof(p) __builtin_threadof((shared void *)p)

If upc_threadof() must also have external linkage, then it must/should have a 
prototype as well.  That prototype will only express "shared void *" as the 
type of the argument.  Thus, although the macro can transparently recast, the 
external prototype cannot.

Further, the __builtin_threadof() implementation in GCC also requires a 
prototype and the recast is still subject to the "cast drops qualifiers" 
warning if the actual argument is a pointer to a target that is either 
strict/relaxed qualified.

I view strict/relaxed as analogous to 'restrict' in that the affect the 
behavior of the access via a pointer, but are not subject to qualifier checks 
when matching actual argument values against formal parameters.

Original comment by gary.funck on 22 May 2012 at 3:33

[GoogleCodeExporter]

The more I learn about this proposal, the less my objections become.
Having seen that an "alternative proposal" is to ignore "strict" and "relaxed" 
qualifiers when converting to (shared void *), this proposal looks like the 
much better choice.  I said previously only that I didn't see the need for the 
change (not that the change would be bad), and now I do understand the need.

I am now in favor of this proposal: the functions 
upc_{threadof,phaseof,addrfield}() for dissection of a pointer-to-shared should 
be made unary operators.

So, does this mean that, like sizeof() and friends, they would NOT evaluate the 
expression?
Not evaluating could be a problem if any existing UPC code has calls with 
side-effects in the argument.  However, evaluating the expression might be less 
consistent with the sizeof family of operators, leading to errors in which the 
UPC code was not expecting the expression evaluation.  Either way we lose.

Original comment by phhargr...@lbl.gov on 23 May 2012 at 8:09

[GoogleCodeExporter]

I think we have to expect the expression to be evaluated, since there is often 
no way to determine the operation value otherwise.  Consider:

extern shared int *foo(void);
int t = upc_threadof(foo());

The function foo has to be called to obtain a value from which a thread is 
extracted.  Unlike sizeof, which can operate just based on the type returned by 
foo(), upc_threadof needs the value.

There are other unary C operators, such as increment and decrement, that 
perform expression evaluation.  I think most do, actually.  Just not sizeof, 
which has a name similar to these UPC versions.

I am in favor of this proposal.

Original comment by brian.wibecan on 25 May 2012 at 10:05

[GoogleCodeExporter]

I (Paul) had asked:
> So, does this mean that, like sizeof() and friends, they would NOT evaluate 
the expression?

Brian responded:
> I think we have to expect the expression to be evaluated, since there is 
often no way to determine the operation value otherwise.

Of course Brian is 100% right that unlike sizeof() these would be operations on 
a VALUE, not a TYPE.  I am not even sure what I was thinking when I wrote that, 
but can only guess that I had in mind the upc_localsizeof() operator in mind 
(which does operate on a type).

So, I withdraw the question about evaluation of the arguments, and am still in 
favor of this proposal.

Original comment by phhargr...@lbl.gov on 25 May 2012 at 10:19

[GoogleCodeExporter]

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

• Added labels: Spec-1.3

[GoogleCodeExporter]

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

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

[GoogleCodeExporter]

This entire issue bothers me, because the strictness or relaxedness of the 
pointer argument is of course completely irrelevant for these functions as they 
never even dereference the pointer. The fact the referent type is an incomplete 
type (ie void) is already a flag to the type system that the callee will not 
dereference the given pointer (at least not without first casting, which would 
also remove any strict/relax qualifier).

I'm in favor of whatever it takes to make these warnings "go away" for the 
user, since forcing them to insert casts in this case is just silly. I see two 
possibilities:

(1) Make them operators which silently accept any pointer-to-shared argument. 
This works, but just sweeps the problem under the rug where it will re-appear 
for any library functions that accept (shared void*). However, perhaps we don't 
care since the majority of such library functions DO perform dereference, so 
forcing the user to cast might also encourage them to consider the 
strict/relax-ness of the library's accesses, which may be a Good Thing.

(2) Add some special-case specification language stating that strict/relax 
qualifiers are stripped from the top-level referent type when passed as 
arguments to a function prototype specifying a (shared void *) parameter. This 
is similar in spirit to the special case already provided in C99 6.5.2.2-7:

"arguments are implicitly converted, as if by assignment, to the types of the
corresponding parameters, taking the type of each parameter to be the 
unqualified version of its declared type."

Except we'd be dropping a qualifier on the top-level referent type of a 
pointer-to-shared. The motivation is basically "the callee can't reference the 
incomplete type without a cast anyhow". I don't know the implementation impact 
of this approach, but it has the nice effect of extending the usability feature 
to other libraries which we'd never make into operators.

Original comment by danbonachea on 14 Jun 2012 at 11:48

[GoogleCodeExporter]

It's worth noting my option #2 is basically the change proposed by issue #20 
http://code.google.com/p/upc-specification/issues/detail?id=20

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

[GoogleCodeExporter]

A follow up to Dan's comment #10, where he notes that option #2 is the change 
proposed by issue #20, below is a copy of Paul's comment added to issue #20.

"I would argue that dropping this check might not be safe in general.
For instance, passing a strict-qualified pointer-to-shared into the collective 
operations will NOT magically produce strict semantics for the data accesses 
within the collectives.  So, in that case I would want the warning to appear 
unless the user has supplied a cast to silence the warning and thus "sign the 
contract" so-to-speak.

I think that a better response to the upc_threadof() example would be the 
proposal in issue #19 (in the issue tracker, not the original list) to make 
certain functions into operators."

I agree with Paul's point on this issue.  If we use "volatile" as an analog to 
"strict".  For example, if we consider the following "C" example:

#include <stddef.h>

ptrdiff_t
ptr_as_int (void *p)
{
  return (ptrdiff_t) p;
}

volatile int i;

ptrdiff_t result;

int main (void)
{
  result = ptr_as_int (&i);
  return 0;
}

$ gcc -std=c99 t.c
t.c: In function ‘main’:
t.c:15:3: warning: passing argument 1 of ‘ptr_as_int’ discards 
‘volatile’ qualifier from pointer target type [enabled by default]
   result = ptr_as_int (&i);
   ^
t.c:4:1: note: expected ‘void *’ but argument is of type ‘volatile int 
*’
 ptr_as_int (void *p)
 ^

relaxing conversions to (shared void *) so that they drop strict/relaxed seems 
counter to current convention/practice.

Original comment by gary.funck on 29 Jun 2012 at 7:47

[GoogleCodeExporter]

It's worth noting that we are primarily dealing with three different "generic" 
void pointer types here:

1: (shared strict void *)
2: (shared relaxed void *)
3: (shared void *)

the functions in question, and most of the UPC libraries, declare a formal 
parameter using a type similar to type #3. Paul's objection to my proposal is 
concerned with silently converting type #1 into #3. However it seems harmless 
to allow silent conversion from type #2 to type #3, so we could potentially 
solve this by just adding that implicit conversion (silently dropping the 
"relaxed" on a (shared relaxed void *)), and then the nuisance warnings only 
arise when handling strict pointers (presumably a smaller usage case).

As an aside, the existing upc_threadof() and friends should probably include a 
"const" qualifier on the referent type to prevent similar nuisance warnings 
when passing pointer-to-shared-const into these functions.

A third possible solution would be to introduce a new reference-type-qualifier 
to handle this specific case. So in addition to "strict" and "relaxed", we 
could add one called "noaccess" (or something else clever) which would act as 
an explicit wildcard in a formal parameter. Eg:

size_t upc_threadof(shared noaccess void *ptr);

The new reference-type-qualifier is only permitted in the qualifiers for a 
top-level shared-qualified referent type of a formal argument, and asserts that 
the pointer in question is never dereferenced by the callee. In the case of a 
user function, this property should be easily enforceable (although we'd need 
to decide whether to allow the "noaccess" to be cast away, or appear on stack 
variables). 
Because the pointer is never dereferenced, it is safe to implicitly convert 
away strict, relaxed and even const when they appear in the matching actual 
argument. This would make the issue "go away" for users, at the cost of a new 
keyword and a few paragraphs of spec that everyone but compiler writers can 
ignore.

Another note, if we obtain a generic resolution to this issue, it should 
probably also be applied to the upc_castable() library in issue #37, as that 
interface has exactly the same situation of accepting a shared void pointer 
parameter which is never dereferenced by the callee.

Original comment by danbonachea on 3 Aug 2012 at 3:10

[GoogleCodeExporter]

Set default Consensus to "Low".

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

• Added labels: Consensus-Low

[GoogleCodeExporter]

Mass change "Accepted" issues which haven't seen activity for over a month back 
to "New", for telecon discussion.

Original comment by danbonachea on 4 Oct 2012 at 11:36

• Changed state: New

[GoogleCodeExporter]

This issue and related issue 20 were discussed in the 10/5 telecon. The three 
proposed solutions discussed in comments 9 and 12 were considered, but no 
resolution was reached.

Original comment by danbonachea on 7 Oct 2012 at 6:37

[GoogleCodeExporter]

Original comment by danbonachea on 7 Oct 2012 at 6:39

• Now blocking: #20

[GoogleCodeExporter]

In the 10/24 telecon we decided to defer this issue to 1.4.

Original comment by danbonachea on 27 Oct 2012 at 3:32

• Added labels: Milestone-Spec-1.4
• Removed labels: Milestone-Spec-1.3

[GoogleCodeExporter]

Issue 113 has been merged into this issue.

Original comment by danbonachea on 3 Aug 2013 at 4:32