hello Andre,
Le 25/05/2015 12:24, Andre Vehreschild a écrit :
> Hi Paul,
>
> I am not quite happy with the naming of the temporary variable. When I
> initially set the prefix to "atmp" this was because the variable would be an
> array most of the time and because of the number appended to it should be
> unique
> anyway. However I would like to point out that disclosing an internal
> implementation detail of the compiler to a future developer looking at the
> pseudo-code dump will not help (I mean "expr3", here). I would rather use
> "source" as the prefix now that I think of it with some distance to the
> original naming. What do you think?
>
> Now that the deallocate for source's components is in the patch, I understand
> why initially the source= preevaluation for derived types with allocatable
> components was disallowed. Thanks for clarifying that.
>
> I wonder though, if we can't do better...
>
> Please have a look at the attached patch. It not only renames the temporary
> variable from "expr3" to "source" (couldn't help, but do it. Please don't be
> angry :-)), but also adds move semantics to source= expressions for the last
> object to allocate. I.e., when a scalar source= expression with allocatable
> components is detected, then its content is "moved" (memcpy'ed) to the last
> object to allocate instead of being assigned. All former objects to allocate
> are of course handled like before, i.e., components are allocated and the
> contents of the source= expression is copied using the assign. But when a move
> could be done the alloc/dealloc of the components is skipped. With this I hope
> to safe a lot of mallocs and frees, which are not that cheap. In the most
> common
> case where only one object is allocated, there now is only one alloc for the
> components to get expr3 up and one for the object to allocate. We safe the
> allocate of the allocatable components in the object to allocate and the free
> of the source= components. I hope I could make clear what I desire? If not
> maybe a look into the patch might help. What do you think?
>
> The patch of course is only a quick implementation of the idea. Please
> comment, everyone!
>
This looks good to me as well with...
*************** gfc_trans_allocate (gfc_code * code)
*** 5840,5845 ****
--- 5837,5856 ----
}
else
{
+ if (al->next == NULL && needs_comp_dealloc && expr->rank == 0)
+ {
...please use "else if" to avoid changing indentation unnecessarily.
Then OK if it passes the testsuite.
Give Paul a couple of days to comment, before committing.
Mikael
