[Fortran] Fix type decl of coarrays

On Sat, May 28, 2011 at 8:24 AM, Tobias Burnus <burnus@net-b.de> wrote:
> Thanks for Richard for debugging this!
>
> Setting TREE_TYPE in trans-decl.c is nonsense and leads to types of the form
> "_Complex _Complex int". The fix is rather obvious.
>
> Build and regtested on x86-64-linux.
> OK for the trunk?

Note that I thought that instead of

   TYPE_LANG_SPECIFIC (type)

what was probably intended was

-    {
-      type = build_variant_type_copy (etype);
-      TREE_TYPE (type) = etype;
-    }
+    type = build_variant_type_copy (type);

because when copying the element type the following line

   GFC_ARRAY_TYPE_P (type) = 1;

doesn't make too much sense (to me).  And the copying was
probably to avoid clobbering the shared type.

So, please figure out what is really intended here ;)

Richard.

> Tobias
>

Richard Guenther wrote:
> Note that I thought that instead of
>     else
> -    {
> -      type = build_variant_type_copy (etype);
> -      TREE_TYPE (type) = etype;
> -    }
> +    type = build_variant_type_copy (etype);
>
>     GFC_ARRAY_TYPE_P (type) = 1;
>     TYPE_LANG_SPECIFIC (type)
>
> what was probably intended was
>
> -    {
> -      type = build_variant_type_copy (etype);
> -      TREE_TYPE (type) = etype;
> -    }
> +    type = build_variant_type_copy (type);

But that doesn't make sense. The function is declared as:

gfc_get_nodesc_array_type (tree etype, gfc_array_spec * as, gfc_packed 
packed,
                            bool restricted)
{
   tree type;

Thus, if I use "build_variant_type_copy" on "type", I use uninitialized 
memory! What I want to have is the type passed to the function, which is 
"etype" (= type of an array element).

For normal arrays, one creates an ARRAY_TYPE, with elements of type etype.

However, the code in question is for scalar coarrays (rank == 0). For 
the middle end they should be normal scalars of type "etype", but for 
the front end, I need to associate some extra information with the type 
- such as the coarray rank, its cobounds and the associated token 
variable. In order to save this information, I need to create a new type 
variant and save the information in TYPE_LANG_SPECIFIC.

Thus, I maintain that the code with the patch is as intended.

> because when copying the element type the following line
>     GFC_ARRAY_TYPE_P (type) = 1;
> doesn't make too much sense (to me).

That's just for the front end! Scalar coarrays have a dual nature: On 
one hand, they act as arrays such that one can ask for their cobounds 
("ucobound(caf_variable)") and that one can access single elements 
("caf_var[2]") - on the other hand, this arrayness is just restricted to 
the front-end; for the middle end, the type should be a normal scalar. 
Thus, a scalar coarray should act in expressions such as
   a = 7
as a normal scalar variable in local memory. Only for
   a[9] = 7
one accesses remote memory. But that's translated into a function call 
with the pseudocode:
   _gfortran_caf_remote_scalar_assign (token_of_a, image=9, rhs=7)

(If we had a shared memory implementation, one could create a true 
array, where "a =" would be mapped to "a[this_image()]" and "a[7]" would 
be translated as such.)

In any case, it turned out that handling scalar coarrays is similar to 
handling normal descriptorless arrays and descriptorless array coarrays. 
Thus, I use the same machinery with GFC_ARRAY_TYPE_P (...) = 1 - and 
just added a few ifs blocks for the few cases scalar coarrays need to be 
handled differently.

Thanks for looking at the patch!

Tobias

On Sat, May 28, 2011 at 3:35 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Richard Guenther wrote:
>>
>> Note that I thought that instead of
>>    else
>> -    {
>> -      type = build_variant_type_copy (etype);
>> -      TREE_TYPE (type) = etype;
>> -    }
>> +    type = build_variant_type_copy (etype);
>>
>>    GFC_ARRAY_TYPE_P (type) = 1;
>>    TYPE_LANG_SPECIFIC (type)
>>
>> what was probably intended was
>>
>> -    {
>> -      type = build_variant_type_copy (etype);
>> -      TREE_TYPE (type) = etype;
>> -    }
>> +    type = build_variant_type_copy (type);
>
> But that doesn't make sense. The function is declared as:
>
> gfc_get_nodesc_array_type (tree etype, gfc_array_spec * as, gfc_packed
> packed,
>                           bool restricted)
> {
>  tree type;
>
> Thus, if I use "build_variant_type_copy" on "type", I use uninitialized
> memory! What I want to have is the type passed to the function, which is
> "etype" (= type of an array element).
>
> For normal arrays, one creates an ARRAY_TYPE, with elements of type etype.
>
> However, the code in question is for scalar coarrays (rank == 0). For the
> middle end they should be normal scalars of type "etype", but for the front
> end, I need to associate some extra information with the type - such as the
> coarray rank, its cobounds and the associated token variable. In order to
> save this information, I need to create a new type variant and save the
> information in TYPE_LANG_SPECIFIC.
>
> Thus, I maintain that the code with the patch is as intended.

Ah, ok.  I only looked at the context the patch provided and the function
name (which didn't sound like it is co-array specific).

Thanks,
Richard.

>> because when copying the element type the following line
>>    GFC_ARRAY_TYPE_P (type) = 1;
>> doesn't make too much sense (to me).
>
> That's just for the front end! Scalar coarrays have a dual nature: On one
> hand, they act as arrays such that one can ask for their cobounds
> ("ucobound(caf_variable)") and that one can access single elements
> ("caf_var[2]") - on the other hand, this arrayness is just restricted to the
> front-end; for the middle end, the type should be a normal scalar. Thus, a
> scalar coarray should act in expressions such as
>  a = 7
> as a normal scalar variable in local memory. Only for
>  a[9] = 7
> one accesses remote memory. But that's translated into a function call with
> the pseudocode:
>  _gfortran_caf_remote_scalar_assign (token_of_a, image=9, rhs=7)
>
> (If we had a shared memory implementation, one could create a true array,
> where "a =" would be mapped to "a[this_image()]" and "a[7]" would be
> translated as such.)
>
> In any case, it turned out that handling scalar coarrays is similar to
> handling normal descriptorless arrays and descriptorless array coarrays.
> Thus, I use the same machinery with GFC_ARRAY_TYPE_P (...) = 1 - and just
> added a few ifs blocks for the few cases scalar coarrays need to be handled
> differently.
>
> Thanks for looking at the patch!
>
> Tobias
>