Hi all,
I am sorry, I don't get it. So we are trying to help the compiler, i.e. the C++
one, to create a fast gfortran binary. But we don't care about devs that stumble
about the code and ask themselves, "Why is this done (without a comment) so
oddly?"! Furthermore is the code again using convention or inside knowledge:
intmod_sym symbol[] = {
#define NAMED_INTCST(a,b,c,d) { a, b, 0, d },
#define NAMED_UINTCST(a,b,c,d) { a, b, 0, d },
#define NAMED_KINDARRAY(a,b,c,d) { a, b, 0, d },
#define NAMED_DERIVED_TYPE(a,b,c,d) { a, b, 0, d },
#define NAMED_FUNCTION(a,b,c,d) { a, b, c, d },
#define NAMED_SUBROUTINE(a,b,c,d) { a, b, c, d },
So in function and subroutine definitions `c` may become non-constant in the
future and we neither detect it nor care.
#include "iso-fortran-env.def"
{ ISOFORTRANENV_INVALID, NULL, -1234, 0 } };
i = 0;
#define NAMED_INTCST(a,b,c,d) symbol[i++].value = c;
#define NAMED_UINTCST(a,b,c,d) symbol[i++].value = c;
#define NAMED_KINDARRAY(a,b,c,d) i++;
#define NAMED_DERIVED_TYPE(a,b,c,d) i++;
#define NAMED_FUNCTION(a,b,c,d) i++;
#define NAMED_SUBROUTINE(a,b,c,d) i++;
#include "iso-fortran-env.def"
So why not at least adding a comment to document this design decision and I
would also propose to be consequent and also set `c` for functions and
subroutines in the second part. Just to have it similar. You know the
design "principle of least confusion"?!
A confused
Andre
On Wed, 8 Jan 2025 22:41:13 +0100
Mikael Morin <morin-mik...@orange.fr> wrote:
> Le 08/01/2025 à 18:37, Jakub Jelinek a écrit :
> > On Wed, Jan 08, 2025 at 06:23:40PM +0100, Andre Vehreschild wrote:
> >> gcc/fortran/ChangeLog:
> >>
> >> PR fortran/118337
> >> * module.cc (use_iso_fortran_env_module): Prevent additional run
> >> over (un-)signed ints and assign kind directly.
> >> ---
> >> gcc/fortran/module.cc | 13 ++-----------
> >> 1 file changed, 2 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
> >> index de8df05781d..8dc10e1d349 100644
> >> --- a/gcc/fortran/module.cc
> >> +++ b/gcc/fortran/module.cc
> >> @@ -7113,8 +7113,8 @@ use_iso_fortran_env_module (void)
> >> int i, j;
> >>
> >> intmod_sym symbol[] = {
> >> -#define NAMED_INTCST(a,b,c,d) { a, b, 0, d },
> >> -#define NAMED_UINTCST(a,b,c,d) { a, b, 0, d },
> >> +#define NAMED_INTCST(a, b, c, d) {a, b, c, d},
> >> +#define NAMED_UINTCST(a, b, c, d) {a, b, c, d},
> >> #define NAMED_KINDARRAY(a,b,c,d) { a, b, 0, d },
> >> #define NAMED_DERIVED_TYPE(a,b,c,d) { a, b, 0, d },
> >> #define NAMED_FUNCTION(a,b,c,d) { a, b, c, d },
> >> @@ -7122,15 +7122,6 @@ use_iso_fortran_env_module (void)
> >> #include "iso-fortran-env.def"
> >> { ISOFORTRANENV_INVALID, NULL, -1234, 0 } };
> >>
> >> - i = 0;
> >> -#define NAMED_INTCST(a,b,c,d) symbol[i++].value = c;
> >> -#define NAMED_UINTCST(a,b,c,d) symbol[i++].value = c;
> >> -#define NAMED_KINDARRAY(a,b,c,d) i++;
> >> -#define NAMED_DERIVED_TYPE(a,b,c,d) i++;
> >> -#define NAMED_FUNCTION(a,b,c,d) i++;
> >> -#define NAMED_SUBROUTINE(a,b,c,d) i++;
> >> -#include "iso-fortran-env.def"
> >> -
> >
> > I thought the reason was that NAMED_{,U}INTCST c is non-constant
> > while everything else is constant and the source trying to help
> > the C++ compiler to emit decent code for it.
>
> > Though, I think g++ will end up doing pretty much the same thing,
> > split the non-constant parts of the initializer into statements overwriting
> > values in the variable and using 0 for that in the initializer before
> > it is overwritten.
> >
> I have double-checked that, and it doesn't seem to be the case, at least
> according to the .optimized dump. So I'm inclined to prefer the
> two-stages initialization version.
> Maybe we can add an assert making sure that i remain synced through the
> second stage?
> That would be something like gcc_checking_assert (symbol[i].id == a);
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
