On Thu, Jan 09, 2025 at 11:32:35AM +0100, Andre Vehreschild wrote:
> 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
So like this?
I've also added an assert, I think it isn't really needed to assert that
symbol[i].id == a in each case, just that we increment i for all the cases.
2025-01-09 Jakub Jelinek <ja...@redhat.com>
PR fortran/118337
* module.cc (use_iso_fortran_env_module): Add a comment explaining
the optimization performed. Add gcc_checking_assert that i was
incremented for all the elements. Formatting fix.
--- gcc/fortran/module.cc.jj 2025-01-09 08:25:45.648324540 +0100
+++ gcc/fortran/module.cc 2025-01-09 15:12:07.611842917 +0100
@@ -7122,6 +7122,13 @@ use_iso_fortran_env_module (void)
#include "iso-fortran-env.def"
{ ISOFORTRANENV_INVALID, NULL, -1234, 0 } };
+ /* We could used c in the NAMED_{,U}INTCST macros
+ instead of c, but then current g++ expands the initialization
+ as clearing the whole object followed by explicit stores of
+ all the non-zero elements (over 150), while by using 0s for
+ the non-constant initializers and initializing them afterwards
+ g++ will often copy everything from .rodata and then only override
+ over 30 non-constant ones. */
i = 0;
#define NAMED_INTCST(a,b,c,d) symbol[i++].value = c;
#define NAMED_UINTCST(a,b,c,d) symbol[i++].value = c;
@@ -7130,6 +7137,7 @@ use_iso_fortran_env_module (void)
#define NAMED_FUNCTION(a,b,c,d) i++;
#define NAMED_SUBROUTINE(a,b,c,d) i++;
#include "iso-fortran-env.def"
+ gcc_checking_assert (i == (int) ARRAY_SIZE (symbol) - 1);
/* Generate the symbol for the module itself. */
mod_symtree = gfc_find_symtree (gfc_current_ns->sym_root, mod);
@@ -7288,12 +7296,11 @@ use_iso_fortran_env_module (void)
break;
#define NAMED_FUNCTION(a,b,c,d) \
- case a:
+ case a:
#include "iso-fortran-env.def"
- create_intrinsic_function (symbol[i].name, symbol[i].id, mod,
- INTMOD_ISO_FORTRAN_ENV, false,
- NULL);
- break;
+ create_intrinsic_function (symbol[i].name, symbol[i].id, mod,
+ INTMOD_ISO_FORTRAN_ENV, false, NULL);
+ break;
default:
gcc_unreachable ();
Jakub
