Dear Tobias,

>
> I believe that you mean source-expr (i.e. SOURCE= and MOLD=) and not STATUS.

It was late when I wrote the mail :-)


> I somehow liked your draft patch more:

It caused regressions, though!

>
> * The big program which I reduced to the test case in PR 51870 fails with
> the current patch - only the reduced test case of the PR works. The failure
> of the bigger program is - at runt time - a SIGABRT at
> #6  0x409175 in __show_class_MOD___copy_show_class_Show
>
> * It also fixed PR 48705. Your current patch fixes the reduced test case
> (comment 1) of that PR, but no longer the original version, which fails at
> the end of the program ("end program" line) at run time (SIGABRT). Valgrind
> shows:
>  Invalid write of size 8
>    at 0x4009B3: __generic_deferred_MOD___copy_generic_deferred_Vec (in
> /dev/shm/a.out)
>
> (I assume both programs have the same issue.)
>
> Thus, I would prefer if you could have a look at the latter PR.

Hah! OK

>
>
>>  2012-01-22  Paul Thomas<pa...@gcc.gnu.org>
>>        PR fortran/51870
>
>
> Could you also add PR fortran/51943 and PR 51946? (I think those are
> effectively the same examples. Also the full example
> ssdSource/chapter08/puppeteer_f2003 works for me.)

I am happy to comply with that one !

>
>
> +         /* This is the safest way of converting to a compatible
> +            type for use in the allocation.  */
> +         tmp = TYPE_SIZE_UNIT (TREE_TYPE (gfc_index_zero_node));
> +         memsz = fold_convert (TREE_TYPE (tmp), memsz);
>
> How about:
> +         memsz = fold_convert (size_type_node, memsz);

No - it does not work.  It was the first thing that I tried. The
actual representation is character (kind=4), I think, and this cuased
fold_convert to barf.  I chose to go the route above since it is
always going to be correct for any platform.  In fact, tmp =
TYPE_SIZE_UNIT (gfc_array_index_type); would have been still neater.
If trans-types.c generates an appropriate type, I would be curious to
know which it is, since I have encountered this wrinkle before.

>
>
>          /* Determine allocate size.  */
> -         if (al->expr->ts.type == BT_CLASS && code->expr3)
> +         if (al->expr->ts.type == BT_CLASS
> + && code->expr3
> + && memsz == NULL_TREE)
>            {
>
> Indentation looks wrong.

I'll take a look.

>
>
>   for (al = code->ext.alloc.list; al != NULL; al = al->next)
>     {
> ...
> +         class_expr = build_fold_indirect_ref_loc (input_location,
> +                                                   se_sz.expr);
> +         class_expr = gfc_evaluate_now (class_expr, &se.pre);
>
> I have the feeling that you evaluate the function multiple times. Actually,
> for*

Not for SOURCE=.... in this case, the allocate list is only allowed one member.

>
> allocate(kernel, kernel2, mold=executive_producer%create_show() )
>
> I find:
>
>        D.1890 = create_show ();
>        (struct __vtype_show_producer_class_Integrand *) kernel._vptr =
> (struct __vtype_show_producer_class_Integrand *) D.1890._vptr;
>        (void) __builtin_memcpy ((void *) kernel._data, (void *) &create_show
> (), 4);
>
>        D.1892 = create_show ();
>        (struct __vtype_show_producer_class_Integrand *) kernel2._vptr =
> (struct __vtype_show_producer_class_Integrand *) D.1892._vptr;
>        (void) __builtin_memcpy ((void *) kernel2._data, (void *)
> &create_show (), 4);
>
> Thus, one evaluates the function 4 times instead of only once. Additionally,
> MOLD= does not invoke the default initializer (as expected for MOLD=) but
> memcopy (as expected for SOURCE=).

I could fix MOLD but I was attempting to minimise the impact. I'll get on to it.

>
> The memcpy is also wrong. If CLASS(integrand) (of create_show) returned a
> derived type with allocatable components, one had to to a deep copy. As this
> is not known at compile time, a call to vtab->__copy is required.
>
> And a last issue: If one changes in
>   type(show_producer), allocatable :: executive_producer
> the TYPE to CLASS one gets still an ICE in conv_function_val.

As above - we are entering problems that I did not attempt to address.

>
> Tobias
>
> * Ditto for SOURCE=, though there one runs into PR51953 as F2003 only
> allowed one allocate-object.

Indeed.

Thanks for the thorough review.  I'll retire to lick my wounds and fix
MOLD and PR48705.

Paul

Reply via email to