On Tue, Nov 07, 2017 at 08:26:15PM +0200, Janne Blomqvist wrote: > Earlier GFortran used to redefine boolean_type_node, which in the rest > of the compiler means the C/C++ _Bool/bool type, to the Fortran > default logical type. When this redefinition was removed, a few > issues surfaced. Namely, > > 1) PR 82869, where we created a boolean tmp variable, and passed it to > the runtime library as a Fortran logical variable of a different size. > > 2) Fortran specifies that logical operations should be done with the > default logical kind, not in any other kind. > > 3) Using 8-bit variables have some issues, such as > - on x86, partial register stalls and length prefix changes. > - s390 has a compare with immediate and jump instruction which > works with 32-bit but not 8-bit quantities. > > This patch addresses (2) by introducing a type > default_logical_type_node which is used when evaluating Fortran > logical expressions. (3) is addressed by introducing > logical_type_node, a tree representing a logical(kind=4) type which > can be used for compiler-generated temporary > variables. logical_type_node is always 4 bytes. As a side effect, (1) > is also fixed, though there might be some latent bug lurking there > still. > > For x86-64, using the Polyhedron benchmark suite, no performance or > code size difference worth mentioning was observed. > > Regtested on x86_64-pc-linux-gnu. Ok for trunk?
I scanned the patch and it looks ok to me (most a mechanical find/replace operation). One thing those, > diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c > index 78477a9..602d369 100644 > --- a/gcc/fortran/trans-types.c > +++ b/gcc/fortran/trans-types.c > @@ -62,6 +62,12 @@ tree ppvoid_type_node; > tree pchar_type_node; > tree pfunc_type_node; > > +tree default_logical_type_node; > +tree default_logical_true_node; > +tree default_logical_false_node; > +tree logical_type_node; > +tree logical_true_node; > +tree logical_false_node; > tree gfc_charlen_type_node; > > tree gfc_float128_type_node = NULL_TREE; > @@ -1003,6 +1009,15 @@ gfc_init_types (void) > wi::mask (n, UNSIGNED, > TYPE_PRECISION (size_type_node))); > > + > + default_logical_type_node = gfc_get_logical_type > (gfc_default_logical_kind); > + default_logical_true_node = build_int_cst (default_logical_type_node, 1); > + default_logical_false_node = build_int_cst (default_logical_type_node, 0); > + > + logical_type_node = gfc_get_logical_type (4); Can you add a comment to note that the 4 is purposely chosen? A year or so from now, someone might change this to gfc_default_logical_kind without understand/recalling why 4 is used. -- steve
