Le 15/07/2023 à 08:11, Paul Richard Thomas via Fortran a écrit :
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?
I understand it makes it more difficult to get the bigger, high level,
picture. On the other hand, it separates pure reorganization changes
from real changes, and should help to isolate regressions, if any. A
single patch looks to me like it rewrites big chunks with new code,
which is not really the case here.
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.
Honestly, I can't really say. I just carried it around unchanged, as it
was originally, in patch 3/14. By the way, this makes me notice that
the variable name should have been changed from se to tmp_se.
As both pre and post blocks are copied to the final code block, I think
the assertions are not useful and can just be removed.
OK for trunk
Thanks for all the work and the patches.
Thanks for your review, and sorry for the wasted energy in getting a
usable work tree.
