Closed GoogleCodeExporter closed 9 years ago
I don't follow why that's erroneous. The #define BLOCK2 12 is a preprocessor
macro, so the declaration is shared [12] int array[THREADS*12*10]. The 12*10
is an integer constant expression.
Original comment by johnson....@gmail.com
on 5 Oct 2012 at 5:06
Troy wrote:
"I don't follow why that's erroneous. The #define BLOCK2 12 is a preprocessor
macro, so the declaration is shared [12] int array[THREADS*12*10]. The 12*10
is an integer constant expression."
If the this is an equivalent factorization, (THREADS*12)*10, then the entire
array index is expression is not "the threads expression". If the expression
had been written THREADS*(12*10) then clearly the threads expression is THREADS
multiplied by a constant expression and this threads expression is the entire
expression appearing in the index term (domain) of the array declarator.
I will note that we have subsequently changed Clang UPC to use the more
conventional, intuitive interpretation. In this report, I am just highlighting
a potential ambiguity in the specification, for the purposes of discussion and
possible clarification.
Original comment by gary.funck
on 5 Oct 2012 at 5:29
The issue we're dealing with here is that under dynamic threads, the shared
array dimension involving THREADS needs to provably be an expression whose
value is strictly linear in THREADS (and otherwise constant). However, there's
a syntactic question of how "hard" the compiler should have to work to "prove"
this linearity before accepting the declaration as valid.
For example, all of the following declarations have dimensions which are
technically linear in THREADS and satisfy the underlying requirement, but it's
probably not reasonable to expect a UPC type checker to prove this:
shared int x[(THREADS*10)*20];
shared int x[(THREADS*1*1*1)*20];
shared int x[(THREADS+0)*20];
shared int x[(THREADS+100-100)*20];
shared int x[(THREADS*sizeof(char)/sizeof(int8_t)*((int)66.34)-'A')*20];
shared int x[(THREADS+THREADS-THREADS)*THREADS/THREADS];
Consequently, the specification needs to "draw the line" somewhere regarding
what will be accepted, and the current text is INTENDED to state that the
dimension involving THREADS has one of the following forms:
[THREADS]
[THREADS * (integer constant expression)]
[(integer constant expression) * THREADS]
Where "integer constant expression" is a very specific entity defined in C99
6.6, and is grouped as a single subexpression to the multiplication by THREADS,
either explicitly (via parentheses) or implicitly (according to the operator
syntax grouping defined in C99 6.5).
To answer Gary's nitpick, the phrase "the THREADS expression" is INTENDED to
mean exactly "THREADS", not "any expression containing THREADS" , as the latter
would include ridiculous cases like (THREADS+THREADS) or (THREADS*clock()),
which are clearly not what's intended.
By my reading this makes the original example erroneous, because after applying
the operator grouping of C syntax, the whole dimension effectively reads:
[((THREADS*12)*10)]
which does not conform to any of the cases above. This is a bit unintuitive,
but the programmer can easily fix this by re-parenthesizing his expression. The
fact the re-parenthesized expression happens to be mathematically equivalent
(in this case) does not mean the type checker should be required to prove that.
This all seems relatively clear to me in the current wording, although perhaps
Gary could elaborate on the manner of clarification he's suggesting?
Original comment by danbonachea
on 6 Oct 2012 at 8:07
Is there really a decent C compiler out there that can't figure out that all of
Dan's examples are constant integer expressions times THREADS? If I was using
a compiler that could not figure out that
"(x*sizeof(char)/sizeof(int8_t)*((int)66.34)-'A')*20" was "a*x" for some
integer constant a, I would fire my C compiler and get a new one. I do not see
why the same logic should not apply to a UPC compiler.
Original comment by jeff.science@gmail.com
on 7 Oct 2012 at 9:33
Jeff asks:
"Is there really a decent C compiler out there that can't figure out that all
of Dan's examples are constant integer expressions times THREADS?"
For some expressions involving things like THREADS*THREADS/THREADS, it is
important to keep in mind that in a dynamic threads translation environment
that THREADS is either a variable or an expression that will not yield a
compile-time known constant [barring something like JIT compilation unique to
each thread] (and I'm not sure that the spec. says that THREADS is a constant
or invariant expression, but it should). Thus, simplifying THREADS, when it
appears more than once amounts to something like a symbolic mathematic
substitution, and most compilers don't do that sort of optimization in the
front end where language semantics are usually checked.
Not all decent C compilers perform constant folding early in the front-end.
GCC does; Clang doesn't. Clang's front-end (by design) attempts to keep the
initial program AST very close to the original source program syntax. Clang in
fact will track pre-processor macro references to provide better diagnostics,
rather than erasing them away by substituting them in a pre-processor pass
before the front-end sees them. Thus, when Clang sees THREADS*BLOCK2/10 it
literally builds an AST that reads as ((THREADS*BLOCK2)/10). GCC on the other
hand performs pre-processing and constant folding before implementing most
semantic checks. GCC will see (THREADS*120), if we assume for the moment that
THREADS is a variable identifier.
Note that if THREADS is some expression defined by a macro, for example
#define THREADS upc_info->num_threads
that Clang will see the pre-processor symbol THREADS and have the choice of
dealing with the expression in that form or in the substituted form. Note that
if GUPC or any compiler with a GCC-like processing flow ran the preprocessor
first before trying to make the semantic check, its job may become more
difficult. Thus, it is unlikely that THREADS will be implemented as a
preproessor macro at least until after the UPC translator has had a chance to
perform the language required semantic checks.
As Dan points out there are various sound reasons for restricting the types of
expressions that THREADS can appear in within the index (domain) part of an
array declaration. For this reason: certainly THREADS should appear only once
in the expression and then only as the left or right-hand term of a
multiplication, with the other term being provably a compile-time constant.
Assuming some compilers can accept more complicated forms of expressions: this
would argue for making such behavior implementation-defined. Otherwise, more
complicated expressions should be defined as erroneous.
I am in favor of simplifying and making the more complicated form of expression
erroneous; such as that shown in the issue summary. but that may break some
existing programs.
Original comment by gary.funck
on 7 Oct 2012 at 10:45
"Is there really a decent C compiler out there that can't figure out that all
of Dan's examples are constant integer expressions times THREADS?"
It's not that any compiler wont *eventually* figure it out, indeed in many
cases compilers are required to perform constant folding down to a value on
integer constant expressions before code generation. The issue is that
depending on compiler architecture, that may not happen until a late pass in
the compilation process, whereas the typechecker generally needs to run
somewhat early and decide whether a program's declarations are legal. Requiring
the compiler to "prove" linearity for all the examples I gave might represent
an undue implementation burden, and IMHO there's really no good motivation to
require it - if the expression truly is linear in THREADS, then the programmer
should be able to concisely express that in an obvious form.
Also note that in the case of typechecking my last example:
shared int x[(THREADS+THREADS-THREADS)*THREADS/THREADS];
the typechecker would need not only standard constant folding, but also
symbolic expression simplification, a capability which is not strictly required
anywhere else in a C/UPC compiler. Here's a more extreme illustration, which is
also linear in THREADS, but would require an unreasonably smart typechecker to
verify:
shared int x[(THREADS+2)*(THREADS+1)-THREADS*THREADS-2];
Like I said, we have to draw the line somewhere :-)
Original comment by danbonachea
on 7 Oct 2012 at 11:01
Closely related to this issue, it occurred to me that we should also clarify
the integer constant expression must eventually evaluate to a positive value,
specifically to prohibit nonsensical dimensions like:
[THREADS*(sizeof(int)-sizeof(int))]
[THREADS*-14]
This is not technically covered by C99 6.7.5.2 in the dynamic threads
environment, because the full dimension is not an integer constant expression
(due to the inclusion of THREADS).
Original comment by danbonachea
on 8 Oct 2012 at 12:28
I'd think that the user should be able to use *any* (positive) expression which
is valid in other contexts where an integer constant is expected, multiplied by
THREADS.
Any C compiler operating in "C89 mode" (and thus NOT supporting VLA) should
already be able to process the arbitrarily "messy" expressions in comment #3 if
they occur as an array dimension (including the case where they occur in a
typedef) *IF* an actual value were substituted for THREADS. So, I strongly
suspect that enough "machinery" is present in the front-end if one is clever
enough in how they approach the problem.
When the expression is provably a constant, there is no problem to be solved.
Otherwise one can the play a game with substituting THREADS=0 and subjecting
the dimension expression to the compiler's "zero_p" type of test. If there is
no "zero_p" provided then one can leverage the checks which are already done
for array dimensions (and layout qualifiers) which must be non-negative:
zero_p(expr) == (is_non_negative(expr) && is_non_negative(-expr))
Additional checks are then required to ensure the expression is linear (can be
simplified to contain exactly one instance of the THREADS identifier) as
opposed to being quadratic or some rational function. Ideally one can examine
the AST and determine if THREADS already occurs exactly once. If NOT, (which
should be rare but seems to be the case most worrisome to Gary and Dan), then
we can either use "symbolic" or "numeric" approaches to the problem.
Given any (even limited) symbolic capabilities to simplify, one can apply the
simplification before the "check AST for exactly one instance of THREADS" test
before resorting to "numeric" methods. IF there are not sufficient symbolic
capabilities in the compiler to prove linearity, then some more "games"
substituting THREADS==1 and THREADS==2 will allow one to "support" a conclusion
of linearity again using only constant folding and
zero_p( expr(2) - 2*expr(1) )
That doesn't "prove" linearity in general, but if one can determine that the
THREADS identifier occurs N times in the AST, then N+1 "samples" (of which
THREADS==0 is one) *is* sufficient to prove linearity (I know this is true if
expr is a polynomial, but am less sure about "f(THREADS)/g(THREADS)" sorts of
cases).
So, I'd *hope* that allowing this full generality should not impose a
significant implementation burden.
I am not claiming this is TRIVIAL, just that it NOT IMPOSSIBLE.
Original comment by phhargr...@lbl.gov
on 8 Oct 2012 at 11:30
Just to prove to myself that I am not delusional, I've implemented in Berkeley
UPC the scheme described above to test for linearity via "zero_p" and
substitutions of small integers for THREADS in the expression.
It pretty much works with a couple caveats, for the cases in comment #3.
1) The case
shared int x[(THREADS*sizeof(char)/sizeof(int8_t)*((int)66.34)-'A')*20];
is mis-parenthesized and actually should be
shared int x[[THREADS*sizeof(char)/sizeof(int8_t)*((int)66.34-'A'))*20];
if it is to meet the THREADS*constant constraint. Once fixed, this is one BUPC
could already handle.
2) The case
shared int x[(THREADS+THREADS-THREADS)*THREADS/THREADS];
doesn't work because of the 0/0 in the evaluation throws things off.
Of course "(THREADS-N)/(THREADS-N)" could appear, showing that 0 is not
"special".
However, the extra case given in comment #6:
shared int x[(THREADS+2)*(THREADS+1)-THREADS*THREADS-2];
"just works" as my new code counts 4 instances of THREADS in the AST and thus
evaluates the expression at THREADS=0,1,2,3,4,5,6 to prove its linearity.
Original comment by phhargr...@lbl.gov
on 9 Oct 2012 at 2:26
Paul said:
"That doesn't "prove" linearity in general, but if one can determine that the
THREADS identifier occurs N times in the AST, then N+1 "samples" (of which
THREADS==0 is one) *is* sufficient to prove linearity "
I believe your proposed zero_p test is predicated on an assumption that the
dimension expression being tested is a continuous function, which is not
guaranteed for invalid declarations (that must nevertheless be detected).
Here's a few examples of invalid (non-linear) declarations, that I believe pass
your "zero_p" test (false positives):
ex1: shared int x[THREADS*THREADS > 10000 ? THREADS * 16 : THREADS * 15];
Declarations similar to ex1 can be made arbitrarily complicated to defeat the
use of an additional finite number of samples. Here's another where the THREADS
expression only appears once:
ex2: shared int x[THREADS == 1 ? 1 : 2]; // iscontinuity that makes this
expression illegal for THREADS >= 3
It's also not sufficient to simply prohibit the conditional operator, because
similar discontinuites can be created through the use of integer roundoff:
ex3: shared int x[THREADS + (int)(THREADS/1024)]; // discontinuity that makes
this expression illegal only appears at THREADS >= 1024
or with bitwise operators:
ex4: shared int x[THREADS & 0xFF]; // discontinuity that makes this expression
illegal only appears at THREADS >= 256
or with integer overflow:
ex5: shared int x[(THREADS * (INT_MAX / 256))/(INT_MAX/256)]; // this one is
questionable even for a C declaration
Here's a declaration which IS linear for all valid instances of THREADS, but I
suspect triggers a false negative in your test:
ex3: shared int x[THREADS <= 0 ? -20 : THREADS]
Note these declarations are all fully valid in the static threads environment,
but must be prohibited (and reliably rejected by any typechecker) for the
dynamic threads environment.
Original comment by danbonachea
on 9 Oct 2012 at 2:03
This discussion has become somwhat obscure, and I think it would help to
introduce some formalisms to improve the vocabulary of discourse.
----------------------------------------------
Assume we are typechecking an array declaration in the dynamic threads
environment, which has the following form after typedef expansions:
shared [B] T array[expr1][expr2]...[exprN]
Further, assume B >= 1 (not indefinitely blocked)
Define the expression DIMPROD:
DIMPROD := expr1 * expr2 * ... * exprN
----------------------------------------------
Define the predicate CORRECTNESS-PROPERTY:
CORRECTNESS-PROPERTY := There exists a positive integer constant LOCALSZ such that
For All values of THREADS in [1,inf)
(DIMPROD / THREADS) == LOCALSZ
This is the property that we wish to guarantee the implementation.
Specifically, this is the necessary and sufficient condition which enables a
dynamic threads implementation that statically allocates per-thread components
of all static shared objects as constant-sized symbols in the .bss segment of
the executable (without overallocating by more than B*sizeof(T) per thread).
----------------------------------------------
Define the CURRENT-CONSTRAINT:
In the dynamic threads environment, the typechecker shall validate that for all
shared array types declared by the program, the identifier THREADS shall appear
exactly once in DIMPROD, and furthermore the dimension expression (dimI) which
includes THREADS has exactly one of the following forms:
[THREADS]
[THREADS * (integer constant expression)]
[(integer constant expression) * THREADS]
Where "integer constant expression" is the very specific entity defined in C99
6.6, is additionally constrainted to have positive value, and is grouped as a
single subexpression to the multiplication by THREADS, either explicitly (via
parentheses) or implicitly (according to the operator syntax grouping defined
in C99 6.5).
CURRENT-CONSTRAINT is the requirement that was INTENDED by the current spec
wording - it's arguable whether or not the current spec phrasing makes this
unambiguously clear.
----------------------------------------------
THEOREM 1: CURRENT-CONSTRAINT is sufficient to guarantee CORRECTNESS-PROPERTY.
Proof: Consider the three permitted forms of THREADS in a dimension expression:
Case 1, [THREADS]:
For this case, DIMPROD = expr1 * expr2 * .. * THREADS * ... * exprN
Case 2, [THREADS * (integer constant expression)]:
For this case, DIMPROD = expr1 * expr2 * .. * THREADS * (integer constant expression) * ... * exprN
Case 3, [(integer constant expression) * THREADS]:
For this case, DIMPROD = expr1 * expr2 * .. * (integer constant expression) * THREADS * ... * exprN
In all cases, each of the expressions expr1, expr2, ..., exprN and (integer
constant expression) are required by C99 and CURRENT-CONSTRAINT to have a value
that can be evaluated at compile time and results in a positive integer
constant. Let:
LOCALSZ = expr1 * expr2 * .. * (integer constant expression) * ... * exprN (omit "integer constant expression" for Case 1)
By the rules of basic arithmetic (multiplication), the value of LOCALSZ is a
positive integer constant. Furthermore, again by basic arithmetic
(cancellation), for all values of THREADS the expression DIMPROD / THREADS is
equivalent to the expression used to define LOCALSZ. Hence by the identity of
LOCALSZ, the CORRECTNESS-PROPERTY is proven.
----------------------------------------------
By Theorem 1, CURRENT-CONSTRAINT is *sufficient* to imply CORRECTNESS-PROPERTY,
but is not *necessary* - looser constraints are possible which still satisfy
CORRECTNESS-PROPERTY, but its arguable what implementation burden they impose
upon the typechecker.
Summarizing resolutions for this issue:
-------------------------------------
Option 1: "No change": Agree that CURRENT-CONSTRAINT is acceptable and that the
current spec wording adequately and unambiguously describes CURRENT-CONSTRAINT,
so no further action is required.
Option 2: "Clarify": Agree that CURRENT-CONSTRAINT is acceptable, but take
action to clarify the current spec wording to ensure it adequately and
unambiguously describes CURRENT-CONSTRAINT.
Option 3: "Minimal Loosen": Decide to loosen CURRENT-CONSTRAINT by a minimal
amount to allow examples like comment 0, and update the spec wording. For
example require that dimI can be transformed into a form that satisfies
CURRENT-CONSTRAINT by applying associative and commutative transformations on
top-level multiplication operators in dimI. This can easily be proven to
satisfy CORRECTNESS-PROPERTY, and it sounds like this is what some compilers
have already implemented.
Option 4: "Strong Loosen": Decide to loosen CURRENT-CONSTRAINT to some new
predicate (yet to be formalized) which still involves only a single appearance
of THREADS in DIMPROD, but allows more latitude in the dimI expression
(allowing some more operators in the expression tree as a parent to THREADS),
AND can still be formally proven to imply CORRECTNESS-PROPERTY. I would want to
see a formal proof similar to Theorem 1 for any proposed predicate before
considering this option. Also, some handwaving would be needed to argue that it
doesn't impose a significant additional implementation burden for the
typechecker to validate.
Option 5: "Maximally Loosen": Similar to Option 3, but additionally allow dimI
to be an arbitrary expression tree combining THREADS (or possibly multiple
copies of THREADS) and integer constant expressions, provided the resulting
expression behavior satisfies CORRECTNESS-PROPERTY. I have argued this
constraint would in general require full symbolic manipulation by the
typechecker to perform validation, which is a significant departure from the
"spirit" of C's type system, and would constitute an unacceptable
implementation burden.
My 2 cents: I'm in favor of option 1 or 2, but could be convinced to accept
option 3 as a usability enhancement if that is the consensus. I would be
opposed to options 4 and 5 on the basis of complexity and implementation burden
that provides no substantial benefit to users.
Original comment by danbonachea
on 9 Oct 2012 at 2:07
I am in favor of clarification on this issue (having had my idea in comments 8
and 9 squashed by Dan's comment 10). I agree that "CURRENT-CONSTRAINT"
expresses the same thing as the current text, but feel that as written now it
can be misinterpreted.
On a related topic, should we be clear that a compiler MUST NOT accept looser
forms? That would avoid a more permissive compiler from leading a user down
the path of writing non-portable code.
Original comment by phhargr...@lbl.gov
on 9 Oct 2012 at 5:07
"On a related topic, should we be clear that a compiler MUST NOT accept looser
forms? That would avoid a more permissive compiler from leading a user down
the path of writing non-portable code."
The spec wording under review appears in a Constraint section, so this is
already implied by C99 5.1.1.3:
"A conforming implementation shall produce at least one diagnostic message
(identified in an implementation-defined manner) if a preprocessing translation
unit or translation unit contains a violation of any syntax rule or constraint,
even if the behavior is also explicitly specified as undefined or
implementation-defined."
So UPC 1.2 compilers that silently accept more general dimension expressions
are technically non-compliant. I see no need to explicitly restate that, as it
applies to every constraint in the specification. On the other hand, there
probably SHOULD be multiple tests in a good compiler conformance test suite
that exercise all the corner cases for this constraint and validate that every
test pass/fails compilation when it should.
Original comment by danbonachea
on 9 Oct 2012 at 7:16
Original comment by danbonachea
on 17 Jan 2013 at 7:05
Here's a proposal to specify the clarification suggested in comment 11, option
2:
Change this wording:
"Further, the THREADS expression shall only occur either alone or when
multiplied by an integer constant expression"
to:
"Further, the THREADS keyword shall only occur either alone or when multiplied
by an integer constant expression (as defined in [ISO/IEC00 Sec. 6.6]) with
positive value."
Also, we can augment the examples appearing further in the section to
demonstrate the implications (new entries highlighted with <<<<):
EXAMPLE 1: declarations allowed in either static THREADS or dynamic THREADS
translation environments:
shared int x [10*THREADS];
shared int x [THREADS*(100*20)]; <<<<
shared [] int x [10];
EXAMPLE 2: declarations allowed only in static THREADS translation environment:
shared int x [10+THREADS];
shared [] int x [THREADS];
shared int x [10];
shared int x [THREADS][4*THREADS]; <<<<
shared int x [THREADS*THREADS]; <<<<
shared int x [THREADS*100*20]; <<<<
Original comment by danbonachea
on 17 Jan 2013 at 6:15
Set critical priority on the unresolved technical issues slated for 1.3 which
are blocking progress on ratification.
Original comment by danbonachea
on 24 Jan 2013 at 7:40
I concur with the proposal in Comment 15.
Note that per our telecon, that this example likely currently compiles without
error for GCC-based front-ends, due to constant folding in the front-end:
shared int x [THREADS*100*20];
and this same example which used to be diagnosed as an error under Clang UPC
will necessarily be made to fail again. Adjustments will need to be made to
various UPC compilers (probably some test programs, and perhaps some
applications) to bring them into compliance with this change in the
specification.
Original comment by gary.funck
on 28 Jan 2013 at 9:39
Official proposal for issues 94&95 mailed on 2/4/13:
--- upc-language.tex (revision 199)
+++ upc-language.tex (working copy)
@@ -665,18 +679,32 @@
{\bf Constraints}
-\np When a UPC program is translated in the {\em dynamic
- THREADS} environment and an array with shared-qualified elements
- is declared with definite blocksize, the THREADS expression shall
- occur exactly once in one dimension of the array declarator
- (including through typedefs). Further, the THREADS expression
+\np
+ When a UPC program is translated in the {\em dynamic THREADS} environment,
+ \xreplaced[id=DB]{95}{%
+ every {\em declarator} or {\em type-name} containing an array type with
+ shared-qualified elements and definite blocksize shall include the THREADS
keyword
+ }{%
+ and an array with shared-qualified elements
+ is declared with definite blocksize, the THREADS expression shall occur
+ }%
+ exactly once in one dimension of the array declarator
+ (including through typedefs).
+ \xreplaced[id=DB]{94}{%
+ Further, the THREADS keyword shall only occur either alone
+ or when multiplied by an integer constant expression
+ (as defined in [ISO/IEC00 Sec. 6.6]) with positive value.
+ }{%
+ Further, the THREADS expression
shall only occur either alone or when multiplied by an integer
- constant expression.\footnote{In the {\em static THREADS} environment
+ constant expression.
+ }%
+ \footnote{In the {\em static THREADS} environment
THREADS is an integer constant expression, and is therefore valid in
all dimensions.}
\np \xadded[id=DB]{15}{
- The THREADS expression
+ The THREADS keyword
shall not appear anywhere in the declarator of a shared array with
indefinite blocksize under the {\em dynamic THREADS} environment.
}
@@ -704,19 +732,31 @@
\np EXAMPLE 1: declarations allowed in either {\em static THREADS} or
{\em dynamic THREADS} translation environments:
+\cbstart
\begin{verbatim}
shared int x [10*THREADS];
+ shared int x [THREADS*(100*20)];
shared [] int x [10];
\end{verbatim}
+\cbend
\np EXAMPLE 2: declarations allowed only in {\em static THREADS} translation
environment:
+\cbstart
\begin{verbatim}
shared int x [10+THREADS];
shared [] int x [THREADS];
shared int x [10];
+ shared int x [THREADS][4*THREADS];
+ shared int x [THREADS*THREADS];
+ shared int x [THREADS*100*20];
+ shared int (**p)[THREADS][THREADS];
+ typedef shared int (*t)[THREADS][13][THREADS];
+ shared void *p = (shared int (*)[THREADS][THREADS])q;
\end{verbatim}
+\cbend
+\xchangenote[id=DB]{94}{SEVEN NEW EXAMPLES ADDED ABOVE}
\np EXAMPLE 3: declaration of a shared array
Original comment by danbonachea
on 4 Feb 2013 at 5:46
Committed as SVN r214
Original comment by danbonachea
on 30 Apr 2013 at 8:02
Ratified in the 5/22 telecon.
Original comment by danbonachea
on 3 Aug 2013 at 3:55
Original issue reported on code.google.com by
gary.funck
on 5 Oct 2012 at 5:03