On Mon, Dec 12, 2016 at 9:39 PM, Janne Blomqvist <blomqvist.ja...@gmail.com> wrote: > On Mon, Dec 12, 2016 at 7:39 PM, Andre Vehreschild <ve...@gmx.de> wrote: >> Hi Janne, >> >> I found that you are favoring build_int_cst (size_type_node, 0) over >> size_zero_node. Is there a reason to this? > > Yes. AFAIU size_zero_node is a zero constant for sizetype which is not > the same as size_type_node. > > AFAIK the difference is that size_type_node is the C size_t type, > whereas sizetype is a GCC internal type used for address expressions. > On a "normal" target I understand that they are the same size, but > there are some slight differences in semantics, e.g. size_type_node > like C unsigned integer types is defined to wrap around on overflow > whereas sizetype is undefined on overflow. > > I don't know if GCC supports some strange targets with some kind of > segmented memory where the size of sizetype would be different from > size_type_node, but I guess it's possible in theory at least. > > That being said, now that you mention in I should be using > build_zero_cst (some_type_node) instead of > build_int_cst(some_type_node, 0). There's also build_one_cst that I > should use. > >> Furthermore did I have to patch this: >> >> diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c >> index 585f25d..f374558 100644 >> --- a/gcc/fortran/dump-parse-tree.c >> +++ b/gcc/fortran/dump-parse-tree.c >> @@ -465,7 +465,7 @@ show_expr (gfc_expr *p) >> break; >> >> case BT_HOLLERITH: >> - fprintf (dumpfile, "%dH", p->representation.length); >> + fprintf (dumpfile, "%zdH", p->representation.length); >> c = p->representation.string; >> for (i = 0; i < p->representation.length; i++, c++) >> { >> >> to bootstrap on x86_64-linux/f23. > > Ah, thanks for the catch. I'll fix it by using HOST_WIDE_INT_PRINT_DEC > since I'll have to change gfc_charlen_t to be a typedef form > HOST_WIDE_INT (see my answer to FX). > >> And I have this regression: >> >> FAIL: gfortran.dg/allocate_deferred_char_scalar_1.f03 -O1 (test for excess >> errors) >> >> allocate_deferred_char_scalar_1.f03:184:0: >> >> p = '12345679' >> >> Warning: '__builtin_memcpy' writing 8 bytes into a region of size 5 overflows >> the destination [-Wstringop-overflow=] >> allocate_deferred_char_scalar_1.f03:242:0: >> >> p = 4_'12345679' >> >> Warning: '__builtin_memcpy' writing 32 bytes into a region of size 20 >> overflows >> the destination [-Wstringop-overflow=] > > I'm seeing that too, but I assumed they would be fixed by Paul's > recent patch which I don't yet have in my tree yet due to the git > mirror being stuck.. > >> Btw, the patch for changing the ABI of the coarray-libs is already nearly >> done. >> I just need to figure that what the state of regressions is with and without >> my >> change. > > Thanks. > > I'll produce an updated patch with the changes discussed so far. > > > -- > Janne Blomqvist
Hi, attached is the updated patch that applies on top of the original. I didn't do the charlen_zero_node etc, I just fixed the relatively few places in my previous patch rather than everywhere in the entire frontend. Now that the git mirror is working again, I see though those warnings with -O1 from gfortran.dg/allocate_deferred_char_scalar_1.f03 are still there, and Paul's patch didn't get rid of them. :(. I have looked at the tree dumps, however, and AFAICS it's a bogus warning. Ok for trunk? -- Janne Blomqvist
diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c index 585f25d..7aafe54 100644 --- a/gcc/fortran/dump-parse-tree.c +++ b/gcc/fortran/dump-parse-tree.c @@ -348,12 +348,10 @@ show_constructor (gfc_constructor_base base) static void -show_char_const (const gfc_char_t *c, int length) +show_char_const (const gfc_char_t *c, gfc_charlen_t length) { - int i; - fputc ('\'', dumpfile); - for (i = 0; i < length; i++) + for (gfc_charlen_t i = 0; i < length; i++) { if (c[i] == '\'') fputs ("''", dumpfile); @@ -465,7 +463,8 @@ show_expr (gfc_expr *p) break; case BT_HOLLERITH: - fprintf (dumpfile, "%dH", p->representation.length); + fprintf (dumpfile, HOST_WIDE_INT_PRINT_DEC "H", + p->representation.length); c = p->representation.string; for (i = 0; i < p->representation.length; i++, c++) { diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 6a05e9e..e82c320 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2067,10 +2067,12 @@ gfc_intrinsic_sym; typedef splay_tree gfc_constructor_base; -/* This should be a size_t. But occasionally the string length field - is used as a flag with values -1 and -2, see - e.g. gfc_add_assign_aux_vars. */ -typedef ptrdiff_t gfc_charlen_t; +/* This should be an unsigned variable of type size_t. But to handle + compiling to a 64-bit target from a 32-bit host, we need to use a + HOST_WIDE_INT. Also, occasionally the string length field is used + as a flag with values -1 and -2, see e.g. gfc_add_assign_aux_vars. + So it needs to be signed. */ +typedef HOST_WIDE_INT gfc_charlen_t; typedef struct gfc_expr { diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 5e2a750..5b05a3d 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3812,10 +3812,10 @@ or GCC's Ada compiler for @code{Boolean}.) For arguments of @code{CHARACTER} type, the character length is passed as hidden argument. For deferred-length strings, the value is passed by reference, otherwise by value. The character length has the type -@code{INTEGER(kind=4)}. Note with C binding, @code{CHARACTER(len=1)} -result variables are returned according to the platform ABI and no -hidden length argument is used for dummy arguments; with @code{VALUE}, -those variables are passed by value. +@code{INTEGER(kind=C_SIZE_T)}. Note with C binding, +@code{CHARACTER(len=1)} result variables are returned according to the +platform ABI and no hidden length argument is used for dummy +arguments; with @code{VALUE}, those variables are passed by value. For @code{OPTIONAL} dummy arguments, an absent argument is denoted by a NULL pointer, except for scalar dummy arguments of type diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c index 6184289..aaeb4a7 100644 --- a/gcc/fortran/match.c +++ b/gcc/fortran/match.c @@ -5782,8 +5782,8 @@ select_intrinsic_set_tmp (gfc_typespec *ts) sprintf (name, "__tmp_%s_%d", gfc_basic_typename (ts->type), ts->kind); else - sprintf (name, "__tmp_%s_%ld_%d", gfc_basic_typename (ts->type), - charlen, ts->kind); + sprintf (name, "__tmp_%s_" HOST_WIDE_INT_PRINT_DEC "_%d", + gfc_basic_typename (ts->type), charlen, ts->kind); gfc_get_sym_tree (name, gfc_current_ns, &tmp, false); gfc_add_type (tmp->n.sym, ts, NULL); diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 150f0c6..fac531f 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -8727,8 +8727,8 @@ resolve_select_type (gfc_code *code, gfc_namespace *old_ns) if (c->ts.u.cl && c->ts.u.cl->length && c->ts.u.cl->length->expr_type == EXPR_CONSTANT) charlen = mpz_get_si (c->ts.u.cl->length->value.integer); - sprintf (name, "__tmp_%s_%ld_%d", gfc_basic_typename (c->ts.type), - charlen, c->ts.kind); + sprintf (name, "__tmp_%s_" HOST_WIDE_INT_PRINT_DEC "_%d", + gfc_basic_typename (c->ts.type), charlen, c->ts.kind); } else sprintf (name, "__tmp_%s_%d", gfc_basic_typename (c->ts.type), diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index 5b4bb2e..179e136 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -250,7 +250,7 @@ gfc_class_len_or_zero_get (tree decl) return len != NULL_TREE ? fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (len), decl, len, NULL_TREE) - : build_int_cst (gfc_charlen_type_node, 0); + : build_zero_cst (gfc_charlen_type_node); } @@ -1042,7 +1042,7 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts, if (DECL_LANG_SPECIFIC (tmp) && GFC_DECL_SAVED_DESCRIPTOR (tmp)) tmp = GFC_DECL_SAVED_DESCRIPTOR (tmp); - slen = build_int_cst (size_type_node, 0); + slen = build_zero_cst (size_type_node); } else { @@ -1089,7 +1089,7 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts, tmp = slen; } else - tmp = build_int_cst (size_type_node, 0); + tmp = build_zero_cst (size_type_node); gfc_add_modify (&parmse->pre, ctree, fold_convert (TREE_TYPE (ctree), tmp)); @@ -1228,7 +1228,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited) if (from != NULL_TREE && unlimited) from_len = gfc_class_len_or_zero_get (from); else - from_len = build_int_cst (size_type_node, 0); + from_len = build_zero_cst (size_type_node); } if (GFC_CLASS_TYPE_P (TREE_TYPE (to))) @@ -1340,7 +1340,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited) tmp = fold_build2_loc (input_location, GT_EXPR, boolean_type_node, from_len, - build_int_cst (TREE_TYPE (from_len), 0)); + build_zero_cst (TREE_TYPE (from_len))); tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, tmp, extcopy, stdcopy); gfc_add_expr_to_block (&body, tmp); @@ -1368,7 +1368,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited) extcopy = build_call_vec (fcn_type, fcn, args); tmp = fold_build2_loc (input_location, GT_EXPR, boolean_type_node, from_len, - build_int_cst (TREE_TYPE (from_len), 0)); + build_zero_cst (TREE_TYPE (from_len))); tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, tmp, extcopy, stdcopy); } @@ -2196,7 +2196,7 @@ gfc_conv_string_length (gfc_charlen * cl, gfc_expr * expr, stmtblock_t * pblock) gfc_conv_expr_type (&se, cl->length, gfc_charlen_type_node); se.expr = fold_build2_loc (input_location, MAX_EXPR, gfc_charlen_type_node, - se.expr, build_int_cst (TREE_TYPE (se.expr), 0)); + se.expr, build_zero_cst (TREE_TYPE (se.expr))); gfc_add_block_to_block (pblock, &se.pre); if (cl->backend_decl) @@ -2268,7 +2268,7 @@ gfc_conv_substring (gfc_se * se, gfc_ref * ref, int kind, /* Check lower bound. */ fault = fold_build2_loc (input_location, LT_EXPR, boolean_type_node, start.expr, - build_int_cst (TREE_TYPE (start.expr), 1)); + build_one_cst (TREE_TYPE (start.expr))); fault = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR, boolean_type_node, nonempty, fault); if (name) @@ -5877,7 +5877,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, tmp = fold_convert (gfc_charlen_type_node, parmse.expr); tmp = fold_build2_loc (input_location, MAX_EXPR, gfc_charlen_type_node, tmp, - build_int_cst (TREE_TYPE (tmp), 0)); + build_zero_cst (TREE_TYPE (tmp))); cl.backend_decl = tmp; } @@ -8111,7 +8111,7 @@ trans_class_vptr_len_assignment (stmtblock_t *block, gfc_expr * le, from_len = gfc_evaluate_now (se.expr, block); } else - from_len = build_int_cst (gfc_charlen_type_node, 0); + from_len = build_zero_cst (gfc_charlen_type_node); } gfc_add_modify (pre, to_len, fold_convert (TREE_TYPE (to_len), from_len)); @@ -8240,7 +8240,7 @@ gfc_trans_pointer_assignment (gfc_expr * expr1, gfc_expr * expr2) gfc_add_modify (&block, lse.string_length, rse.string_length); else if (lse.string_length != NULL) gfc_add_modify (&block, lse.string_length, - build_int_cst (TREE_TYPE (lse.string_length), 0)); + build_zero_cst (TREE_TYPE (lse.string_length))); } gfc_add_modify (&block, lse.expr, @@ -9676,7 +9676,7 @@ trans_class_assignment (stmtblock_t *block, gfc_expr *lhs, gfc_expr *rhs, tmp = fold_build2_loc (input_location, GT_EXPR, boolean_type_node, from_len, - build_int_cst (TREE_TYPE (from_len), 0)); + build_zero_cst (TREE_TYPE (from_len))); return fold_build3_loc (input_location, COND_EXPR, void_type_node, tmp, extcopy, stdcopy); diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index df414ee..5461666 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -7495,9 +7495,8 @@ gfc_conv_associated (gfc_se *se, gfc_expr *expr) = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, arg1->expr->ts.u.cl->backend_decl, - build_int_cst - (TREE_TYPE (arg1->expr->ts.u.cl->backend_decl), - 0)); + build_zero_cst + (TREE_TYPE (arg1->expr->ts.u.cl->backend_decl))); if (scalar) { /* A pointer to a scalar. */ diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 3285ee0..663bb42 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -2955,7 +2955,7 @@ gfc_trans_character_select (gfc_code *code) if (d->low == NULL) { CONSTRUCTOR_APPEND_ELT (node, ss_string1[k], null_pointer_node); - CONSTRUCTOR_APPEND_ELT (node, ss_string1_len[k], build_int_cst (gfc_charlen_type_node, 0)); + CONSTRUCTOR_APPEND_ELT (node, ss_string1_len[k], build_zero_cst (gfc_charlen_type_node)); } else { @@ -2968,7 +2968,7 @@ gfc_trans_character_select (gfc_code *code) if (d->high == NULL) { CONSTRUCTOR_APPEND_ELT (node, ss_string2[k], null_pointer_node); - CONSTRUCTOR_APPEND_ELT (node, ss_string2_len[k], build_int_cst (gfc_charlen_type_node, 0)); + CONSTRUCTOR_APPEND_ELT (node, ss_string2_len[k], build_zero_cst (gfc_charlen_type_node)); } else { @@ -5640,7 +5640,7 @@ gfc_trans_allocate (gfc_code * code) { gfc_init_se (&se, NULL); temp_var_needed = false; - expr3_len = build_int_cst (gfc_charlen_type_node, 0); + expr3_len = build_zero_cst (gfc_charlen_type_node); e3_is = E3_MOLD; } /* Prevent aliasing, i.e., se.expr may be already a @@ -6036,8 +6036,8 @@ gfc_trans_allocate (gfc_code * code) e.g., a string. */ memsz = fold_build2_loc (input_location, GT_EXPR, boolean_type_node, expr3_len, - build_int_cst - (TREE_TYPE (expr3_len), 0)); + build_zero_cst + (TREE_TYPE (expr3_len))); memsz = fold_build3_loc (input_location, COND_EXPR, TREE_TYPE (expr3_esize), memsz, tmp, expr3_esize);