Implement upc_threadof, upc_phaseof, and upc_addrfield as operators

Issue #19 new
Former user created an issue

Originally reported on Google Code with ID 19

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.

Reported by gary.funck on 2012-05-21 16:36:50

Comments (18)

  1. Former user Account Deleted

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

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

    ```

    Reported by `gary.funck` on 2012-05-21 16:43:09

  2. Former user Account Deleted

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

    1. define upc_threadof(p) builtin_threadof((shared void *)p)

    If I am missing something, please let me know.

    ```

    Reported by `phhargrove@lbl.gov` on 2012-05-22 00:02:46

  3. Former user Account Deleted

    ```

    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:

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

    ```

    Reported by `gary.funck` on 2012-05-22 15:33:02

  4. Former user Account Deleted

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

    ```

    Reported by `phhargrove@lbl.gov` on 2012-05-23 20:09:01

  5. Former user Account Deleted

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

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

  6. Former user Account Deleted

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

    Reported by `phhargrove@lbl.gov` on 2012-05-25 22:19:00

  7. Former user Account Deleted

    Reported by `phhargrove@lbl.gov` on 2012-06-01 03:54:43 - Labels added: Spec-1.3

  8. Former user Account Deleted

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

  9. Former user Account Deleted

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

    ```

    Reported by `danbonachea` on 2012-06-14 23:48:12

  10. Former user Account Deleted

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

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

    ```

    Reported by `gary.funck` on 2012-06-29 19:47:58

  11. Former user Account Deleted

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

    ```

    Reported by `danbonachea` on 2012-08-03 15:10:40

  12. Former user Account Deleted

    ``` Set default Consensus to "Low". ```

    Reported by `gary.funck` on 2012-08-19 23:26:19 - Labels added: Consensus-Low

  13. Former user Account Deleted

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

    Reported by `danbonachea` on 2012-10-04 11:36:12 - Status changed: `New`

  14. Former user Account Deleted

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

    Reported by `danbonachea` on 2012-10-07 06:37:07

  15. Former user Account Deleted
    In the 10/24 telecon we decided to defer this issue to 1.4.
    

    Reported by danbonachea on 2012-10-27 03:32:30 - Labels added: Milestone-Spec-1.4 - Labels removed: Milestone-Spec-1.3

  16. Former user Account Deleted
    Issue 113 has been merged into this issue.
    

    Reported by danbonachea on 2013-08-03 04:32:56

  17. Log in to comment