Hi Mikael, I apologise for not being a bit faster with the review. I appreciate that you put a lot of effort into presenting the work in digestible chunks. Perhaps this is a personal perspective but I found it more difficult to absorb than reviewing a single patch. What do others think?
That said, this is a big improvement to the finalization of variable expressions. I can also confirm that the composite patch applies cleanly and regtests without problems. Please either remove or uncomment the line: // gcc_assert (se.pre.head == NULL_TREE && se.post.head == NULL_TREE); I presume that it reflects some case where the assertion failed? If so, it might be prudent to retain the assertion especially in light of: gcc_assert (tmp_se.post.head == NULL_TREE); a bit further down. OK for trunk Thanks for all the work and the patches. Paul On Thu, 13 Jul 2023 at 19:40, Paul Richard Thomas <paul.richard.tho...@gmail.com> wrote: > > Hi Mikael, > > All 14 patches apply cleanly to trunk, which is rebuilding right now > and will regtest this evening. > > I will review the composite patch tomorrow morning and will come back > to you as soon as I can. > > At first sight all is well; perhaps the commented out line can be > dispensed with? > > Many thanks for this. You are to be commended on your fortitude in > putting it all together. The result looks to be considerably neater > and more maintainable. > > If I recall correctly, Tobias was the author of much of this - any comments? > > Regards > > Paul > > > > On Thu, 13 Jul 2023 at 09:53, Mikael Morin via Fortran > <fortran@gcc.gnu.org> wrote: > > > > Hello, > > > > the following patches are abot PR110618, a PR similar to PR92178 from which > > it is cloned. Both are about a problem of dedendencies between arguments, > > when one of them is associated to an allocatable intent(out) dummy, and thus > > deallocated in the process of argument association. > > > > PR110618 exposes a case where the data reference finalization code > > for one argument references deallocated data from another argument. > > The way I propose to fix this is similar to my recent patches for > > PR92178 [1,2] (and is dependent on them). Those patches try to use a data > > reference pointer precalculated at the beginning of the process instead of > > repeatedly evaluating an expression that becomes invalid at some point > > in the generated code. > > > > Unfortunately, the code for finalization is not prepared for this, as it > > only manipulates front-end expressions, whereas the precalculated > > pointer is available as middle-end's generic tree. > > > > These patches refactor the finalization code to ease the introduction > > of the forementioned pre-calculated class container pointer. Basically, > > four expressions are calculated to build the final procedure call: > > the final procedure pointer, the element size, the data reference > > (array) descriptor, and (optionally) the virtual table pointer. Each of > > the four is outlined stepwise to its own separate function in the > > following patches. This abstracts away the generation of these > > expressions and makes it easier to add one other way to generate them. > > This should also make the impact of the changes more > > visible, and regressions easier to spot. > > > > The main changes are the two last patches introducing an additional > > precalculated pointer argument in relevant functions and using them if > > set. Details are in the specific patches. > > > > Each patch has been bubble-bootstrapped and partially tested > > with RUNTESTFLAGS="dg.exp=*final*". > > The complete set has been fully tested on x86_64-pc-linux-gnu. > > OK for master? > > > > [1] https://gcc.gnu.org/pipermail/fortran/2023-July/059582.html > > [2] https://gcc.gnu.org/pipermail/fortran/2023-July/059583.html > > > > Mikael Morin (14): > > fortran: Outline final procedure pointer evaluation > > fortran: Outline element size evaluation > > fortran: Outline data reference descriptor evaluation > > fortran: Inline gfc_build_final_call > > fortran: Add missing cleanup blocks > > fortran: Reuse final procedure pointer expression > > fortran: Push element size expression generation close to its usage > > fortran: Push final procedure expr gen close to its one usage. > > fortran: Inline variable definition > > fortran: Remove redundant argument in get_var_descr > > fortran: Outline virtual table pointer evaluation > > fortran: Factor scalar descriptor generation > > fortran: Use pre-evaluated class container if available [PR110618] > > fortran: Pass pre-calculated class container argument [pr110618] > > > > gcc/fortran/trans-array.cc | 2 +- > > gcc/fortran/trans-expr.cc | 7 +- > > gcc/fortran/trans-stmt.cc | 3 +- > > gcc/fortran/trans.cc | 314 ++++++++++++-------- > > gcc/fortran/trans.h | 9 +- > > gcc/testsuite/gfortran.dg/intent_out_22.f90 | 37 +++ > > 6 files changed, 237 insertions(+), 135 deletions(-) > > create mode 100644 gcc/testsuite/gfortran.dg/intent_out_22.f90 > > > > -- > > 2.40.1 > > > > > -- > "If you can't explain it simply, you don't understand it well enough" > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
