On 07/15/2011 02:16 PM, Tobias Burnus wrote:
if (code->expr1 || code->expr2)
{
Side remark: One actually only needs to take care whether there is a
STAT=. If there is only an ERRMSG=, the code is unreachable as without
STAT= one gets a run-time error, when an error occurs - and if no error
occurs, ERRMSG= is not modified. Thus, one could reduce the code size by
checking only for code->expr1.
Ok.
/* The coarray library already sets the errmsg. */
if (gfc_option.coarray == GFC_FCOARRAY_LIB
&& gfc_expr_attr (expr).codimension)
tmp = build1_v (GOTO_EXPR, label_finish);
else
tmp = build1_v (GOTO_EXPR, label_errmsg);
OK, I understand now why. It is a bit convoluted - and it is due to an
existing bug in GCC. All (allocatable) coarrays - including
(allocatable) scalar coarrays are arrays - and arrays are handled in
gfc_array_allocate.
The code to jump over the next items to the final or error label is only
checked in the "!gfc_array_allocate" loop.
Thus:
- The code for jumping to the label needs to be either in an "else"
branch or moved out of "if" branch.
- In the "if" branch, you can remove all coarray additions - and add a
gcc_assert() to make sure that indeed no coarray enters there.
There are two if-branches and I'm not sure which one you are talking
about. But let me tell you what I think we should do and you can tell me
if we are on the same page:
I think we should move the entire "if (code->expr1 ...)" block outside
the "if (!gfc_array_allocate)" block. In other words, I propose this:
if (!gfc_array_allocate (&se, expr, stat, errmsg, errlen))
{
... allocate scalar ...
}
if (code->expr1)
{
/* The coarray library already sets the errmsg. */
if (gfc_option.coarray == GFC_FCOARRAY_LIB
&& gfc_expr_attr (expr).codimension)
tmp = build1_v (GOTO_EXPR, label_finish);
else
tmp = build1_v (GOTO_EXPR, label_errmsg);
...
}
My thinking is that this error checking applies equally whether the we
use gfc_array_allocate or not. If we call gfc_array_allocate we still
have stat, we still have errmsg, and we still may or may not call the
coarray library. And I see nothing inside gfc_array_allocate that covers
stat= and errmsg=.
What do you think?
Seemingly, the "if (stat !=0) goto ..." for arrays never worked - not in
GCC 4.1, 4.3, 4.4, 4.6 nor in 4.7.
That would make sense if arrays always went into gfc_array_allocate. In
that case, I think that my proposed change above would fix the problem.
PS: Another bug I found when looking at this patch is PR 49775, it is
related to the code, but an independent issue. I think it will probably
better to place it into a different patch. I was wondering whether you
could/would/want to do it after this patch; it should be straight forward.
Yes, I'd like to do that next. It's very much related to what I've been
doing lately. I think I even remember noticing that the code deallocated
the previous array and was wondering why it did that.
Cheers,
Daniel.
--
I'm not overweight, I'm undertall.
