Proposal: pointer-to-shared comparion should not ignore phase

[Former user]

Originally reported on Google Code with ID 73

Section 6.4.2p6 states:

    Two compatible pointers-to-shared which point to the same object
    (i.e. having the same address and thread components)
    shall compare as equal according to == and !=,
    regardless of whether the phase components match.

I queried Paul Hargrove on this, and he offered the following example (which I have
elaborated a bit with printf's and assertions):

#include <upc.h>
#include <stdio.h>
#include <assert.h>

shared [8] int A[8*THREADS];

int main(void)
{
    shared [] int *p = (shared [] int *)&A[3]; // Has phase of 0 by 6.4.2p2
    shared [8] int *q = (shared [8] int *)p; // Still/again has phase==0
    printf ("phase of &A[3] = %u\n", (unsigned)upc_phaseof(&A[3]));
    printf ("phase of p = %u\n", (unsigned)upc_phaseof(p));
    printf ("phase of q = %u\n", (unsigned)upc_phaseof(q));
    printf ("&A[3] - q = %d\n", (int)(&A[3] - q));
    printf ("q     = (%lu,%u,%u)\n",
      (unsigned long)upc_addrfield(q), (unsigned)upc_threadof(q),
      (unsigned)upc_phaseof(q));
    printf ("&A[0] = (%lu,%u,%u)\n",
      (unsigned long)upc_addrfield(&A[0]),
      (unsigned)upc_threadof(&A[0]),
      (unsigned)upc_phaseof(&A[0]));
    printf ("&A[3] = (%lu,%u,%u)\n",
      (unsigned long)upc_addrfield(&A[3]),
      (unsigned)upc_threadof(&A[3]),
      (unsigned)upc_phaseof(&A[3]));
    assert(q == &A[3]);
    return 0;
}

Paul noted that the likely rationale for the rule above in the specification is that
the phase may be reset via a cast, but that as long as the pointer referred to the
same object (and therefore the difference between the pointers being compared is zero)
that it is irrelevant whether their phase offsets match.

GUPC currently compares the full pointer contents inclusive of the phase and that is
incorrect by the above reasoning.

Here is the current GUPC output:

$ upc -fupc-threads-1 compare-pts.upc
$ a.out
phase of &A[3] = 3
phase of p = 0
phase of q = 0
&A[3] - q = 0
q     = (268,0,0)
&A[0] = (256,0,0)
&A[3] = (268,0,3)
a.out: compare-pts.upc:26: main: Assertion `q == &A[3]' failed.

We plan to correct this.  It is possible that other vendor-supplied compilers (eg,
Cray) may not get this right in their current implementation.  Therefore, I'm logging
this as a language issue tagged with "Clarification".

Reported by gary.funck on 2012-07-17 23:41:14

[Former user]

``` After looking it this issue, I am wondering whether 6.4.2p6 should be changed and restated to indicate that pointer-to-shared equality is just that: all components of the PTS must match.

The rationale for the change is this: The following identity is, in my opinion, desirable.

"Given two compatible PTS's p and q, if (p - q) == 0 then p == q."

However, in the example above, although current spec language defines q to be equal to &a[3], the difference "&A[3] - q" (for THREADS == 2) is -3, not 0.

Is the preservation through various casts that reset the phase worth it, when it is counter to a basic identity property?

```

Reported by `gary.funck` on 2012-07-24 18:46:07

[Former user]

``` Regarding the intended identity, it should have read:

"Given two compatible PTS's p and q, if p == q then (p - q) == 0"

My expectation is that the identity is reflexive, thus:

"p == q iff (p - q) == 0"

```

Reported by `gary.funck` on 2012-07-24 20:40:53

[Former user]

``` It bothers me that p - q does not equal 0 for some cases where the only difference is the phase. My inclination is to revise the subtraction algorithm rather than change the definition of equality. ```

Reported by `brian.wibecan` on 2012-07-24 22:33:02

[Former user]

``` More thoughts. "Equal" can mean "point to the same thing" or "can be used interchangeably". The subtraction version of the equality definition assumes the latter definition of "equal". It is not clear to me that saying p == q guarantees that p+1 == q+1; this is not true in C for assignment-compatible pointers to different types.

The other problem we have is pointers-to-void:

shared void *v; shared [8] int *q; ... v == q ???

This must be possible to compare, and it does not require a cast, but it definitely does not correspond to (p - q) == 0.

FWIW, HP UPC says q == &A[3] for Paul's example. ```

Reported by `brian.wibecan` on 2012-07-24 22:58:17

[Former user]

``` Would you please elaborate on the example with (shared void *)?

Per 6.4.3p2:

The casting or assignment from one pointer-to-shared to another in which either the type size or block size differs results in a pointer with a zero phase, unless one of the types is a qualified or unqualified version of shared void*, the generic pointer-to-shared, in which case the phase is preserved unchanged in the resulting pointer value.

Thus, an assignment to (shared void *) preserves phase.

If we have the following:

1. include <upc.h>
2. include <assert.h>

shared [8] int A[8*THREADS];

int main(void) { shared void *v = &A[3]; shared [8] int *q = v; assert ((&A[3] - q) == 0); return 0; }

and variations on this theme.

In a cast that resets phase, I would view the resulting pointer as not pointing to the original object. Their respective pointer component types do not agree in either block size or the size of the pointer component type.

```

Reported by `gary.funck` on 2012-07-24 23:17:00

[Former user]

``` Regarding the example with (shared void *), I am challenging this statement:

"p == q iff (p - q) == 0"

Pointer subtraction is not defined when one or more of the pointers is (shared void

• ). Thus, we have a case where (p - q) cannot possibly equal 0, yet we should still be able to have pointers that are equal.

Regarding phase being equal, here is another example:

extern void dostuff(shared void *, shared void *); shared [12] int *big_data; shared [] int *single_block; single_block = &big_data[12 * tnum]; ... dostuff(&big_data[x], &single_block[y]);

dostuff(shared void *a, shared void *b) { if (a == b) { They match } else { They don't match }

I don't see a compelling reason that the comparison in the dostuff routine should fail just because the phases don't match. I don't see a compelling reason the caller or the routine should reset the phase just to allow the comparison.

Thinking some more about this, I think my objection boils down to this: we almost never care about phase, except in code that directly deals with block-cyclic arrays. Saying that phase matters for comparison for equality, in particular for (shared void *) pointers, means a lot more cases have to care about phase, including code that does not deal directly with block-cyclic arrays, in the off chance that a pointer with non-zero phase is being examined. ```

Reported by `brian.wibecan` on 2012-07-25 19:53:01

[Former user]

``` The current spec very clearly states that equality tests ignore phase. Compliant 1.2 implementations must currently enforce this. No clarification appears to be required to the existing text to express these semantics.

Changing the behavior to include phase in pointer comparison would break backward compatibility and has the potential to break existing codes. As such, this issue is not a "clarification", its a "request for change" and I believe that relegates it to at least 2.0.

Despite some arguments both ways, there doesn't appear to be a strong motivation for making the suggested change. Programs that really want to include phase in the comparison can already easily express this as:

(p == q) && (upc_phaseof(p) == upc_phaseof(q))

so under current semantics the programmer can already easily compute the type of equality he prefers. As such, the danger of breaking backward compatibility does not appear to be justified.

Move to close with NoChange.

```

Reported by `danbonachea` on 2012-08-03 13:11:53 - Labels added: Type-Other, Milestone-Spec-2.0 - Labels removed: Type-Clarification, Milestone-Spec-1.3

[Former user]

From Gary's comment 1, regarding example in comment 0:
"although current spec language defines q to be equal to &a[3], the difference "&A[3]
- q" (for THREADS == 2) is -3, not 0."

I believe this statement is actually false. Here is the exact spec for PTS subtraction,
in both 1.2 and 1.3 working draft:

 When two pointers-to-shared are subtracted, as described in [ISO/IEC00 Sec.
 6.5.6], the result is undefined unless there exists an integer x, representable
 as a ptrdiff_t, such that (pts1 + x) == pts2 AND upc_phaseof(pts1 +
 x) == upc_phaseof(pts2). In this case (pts2 - pts1) evaluates to x.

In the expression "&A[3] - q", where the two pointers point to the same object (and
compare equal under current spec) but have different phase, there is NO integer that
satisfies both clauses of the specified requirement, therefore the result of the subtraction
is undefined in this case. This was an intentional design decision - when you're applying
operators on two type-compatible but "out-of-phase" pointers into the same array, you're
already in a dangerous (aka very convoluted) place, and we want to strongly discourage
that as a programming practice.

Amongst other things, the restriction prohibits subtraction operations which are not
"symmetrical" with addition. Specifically, it ensures that if (p1 - p2) is well-defined
then the following property always holds:  p2 + (p1 - p2) == p1  This symmetry is a
very basic one which most programmers take for granted, so we want to prevent them
from straying into situations where it would be violated.

Furthermore, contrary to some of the discussion above, the requirement also ensures
the property that (p1 - p2) == 0 implies p1 == p2. The converse implication is false
because of the out-of-phase case (as in Gary's example), where the pointers compare
equal but the result of the subtraction is undefined. However, the converse DOES hold
in all cases where the subtraction operation is well-defined. So the statement "p ==
q iff (p - q) == 0" IS ALREADY TRUE for all cases where the subtraction provides a
well-defined result (ie you didn't just get zero by accident as your undefined result).

Brian's comment 6 gives a good example of why we want pointer equality to ignore phase
(ie mean "points to the same thing" rather than "can be used interchangeably"). While
we discourage the programming practice of type-compatible out-of-phase pointers, we
*encourage* the use of pointers with *different* blocksize to manipulate the same data
(most commonly to reference one thread's slice of a distributed shared array). The
ability to cast two such type-incompatible pointers to (shared void *) and compare
them for equality is often handy. In that situation the programmer is interested in
"points to the same thing" and not even thinking about pointer arithmetic (nor is it
even permitted on shared void *), so forcing them to reset the phase to get a valid
comparison is unintuitive. The "can be used interchangeably" semantic would not even
be useful in this case, because first it would require additional casts back to a complete
pointer type that allows arithmetic, and second that cast would often be erroneous
due to the "phase must be in range of the target type when casting" rule.

Also reiterating from comment 7, the user can already easily express the (less commonly
useful?) "can be used interchangeably" flavor of equality by simply writing: (p ==
q) && (upc_phaseof(p) == upc_phaseof(q))  This is already very concise, so I don't
see a motivation to break backward compatibility to "shorten" this, even if we decided
it was desirable to redefine equality (which I think it's not). 

We have use cases demonstrating the current "points to the same thing" flavor of equality
is useful. 
If we had use cases demonstrating the "can be used interchangeably" flavor of equality
is important, we could consider providing an additional operator to test it (eg p1
=== p3), but the two flavors only seem to significantly differ in cases that combine
type-compatible out-of-phase pointers, which are already Considered Harmful.

I invite further discussion from Gary and others, but otherwise all signs seem to indicate
this proposed change should be rejected.

Reported by danbonachea on 2012-11-05 15:18:14 - Status changed: Rejected