On Sat, Sep 21, 2024 at 2:49 AM Jason Merrill <ja...@redhat.com> wrote:
>
> Tested x86_64-pc-linux-gnu. OK for trunk?
>
> -- 8< --
>
> We've been using -Wno-narrowing since gcc 4.7, but at this point narrowing
> diagnostics seem like a stable part of C++ and we should adjust.
>
> This patch changes -Wno-narrowing to -Wno-error=narrowing so that narrowing
> issues will still not break bootstrap, but we can see them.
>
> The rest of the patch fixes the narrowing warnings I see in an
> x86_64-pc-linux-gnu bootstrap. In most of the cases, by adjusting the types
> of various declarations so that we store the values in the same types we
> compute them in, which seems worthwhile anyway. This also allowed us to
> remove a few -Wsign-compare casts.
>
> The one place I didn't see how best to do this was in
> vect_prologue_cost_for_slp: changing const_nunits to unsigned int broke the
> call to TYPE_VECTOR_SUBPARTS (vectype).is_constant (&const_nunits), since
> poly_uint64.is_constant wants a pointer to unsigned HOST_WIDE_INT. So I
> added casts in that one place. Not too bad, I think.
>
> gcc/ChangeLog:
>
> * configure.ac (CXX_WARNING_OPTS): Change -Wno-narrowing
> to -Wno-error=narrowing.
> * configure: Regenerate.
> * config/i386/i386.h (debugger_register_map)
> (debugger64_register_map)
> (svr4_debugger_register_map): Make unsigned.
> * config/i386/i386.cc: Likewise.
> * diagnostic-event-id.h (diagnostic_thread_id_t): Make int.
> * vec.h (vec::size): Make unsigned int.
> * ipa-modref.cc (escape_point::arg): Make unsigned.
> (modref_lattice::add_escape_point): Use eaf_flags_t.
> (update_escape_summary_1): Use eaf_flags_t, && for bool.
> * pair-fusion.cc (pair_fusion_bb_info::track_access):
> Make mem_size unsigned int.
> * pretty-print.cc (format_phase_2): Cast va_arg to char.
> * tree-ssa-loop-ch.cc (ch_base::copy_headers): Make nheaders
> unsigned, remove cast.
> * tree-ssa-structalias.cc (push_fields_onto_fieldstack):
> Make offset unsigned, remove cast.
> * tree-vect-slp.cc (vect_prologue_cost_for_slp): Add cast.
> * tree-vect-stmts.cc (vect_truncate_gather_scatter_offset):
> Make scale unsigned.
> (vectorizable_operation): Make ncopies unsigned.
> * rtl-ssa/member-fns.inl: Make num_accesses unsigned int.
> ---
> gcc/config/i386/i386.h | 6 +++---
> gcc/diagnostic-event-id.h | 2 +-
> gcc/vec.h | 2 +-
> gcc/config/i386/i386.cc | 6 +++---
> gcc/ipa-modref.cc | 13 +++++++------
> gcc/pair-fusion.cc | 2 +-
> gcc/pretty-print.cc | 2 +-
> gcc/tree-ssa-loop-ch.cc | 6 +++---
> gcc/tree-ssa-structalias.cc | 6 +++---
> gcc/tree-vect-slp.cc | 3 ++-
> gcc/tree-vect-stmts.cc | 7 ++++---
> gcc/configure.ac | 3 +--
> gcc/rtl-ssa/member-fns.inl | 3 ++-
> gcc/configure | 7 +++----
> 14 files changed, 35 insertions(+), 33 deletions(-)
>
> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> index c1ec92ffb15..751c250ddb3 100644
> --- a/gcc/config/i386/i386.h
> +++ b/gcc/config/i386/i386.h
> @@ -2091,9 +2091,9 @@ do {
> \
> #define DEBUGGER_REGNO(N) \
> (TARGET_64BIT ? debugger64_register_map[(N)] : debugger_register_map[(N)])
>
> -extern int const debugger_register_map[FIRST_PSEUDO_REGISTER];
> -extern int const debugger64_register_map[FIRST_PSEUDO_REGISTER];
> -extern int const svr4_debugger_register_map[FIRST_PSEUDO_REGISTER];
> +extern unsigned int const debugger_register_map[FIRST_PSEUDO_REGISTER];
> +extern unsigned int const debugger64_register_map[FIRST_PSEUDO_REGISTER];
> +extern unsigned int const svr4_debugger_register_map[FIRST_PSEUDO_REGISTER];
>
> /* Before the prologue, RA is at 0(%esp). */
> #define INCOMING_RETURN_ADDR_RTX \
> diff --git a/gcc/diagnostic-event-id.h b/gcc/diagnostic-event-id.h
> index 8237ba34df3..06985d23c12 100644
> --- a/gcc/diagnostic-event-id.h
> +++ b/gcc/diagnostic-event-id.h
> @@ -67,6 +67,6 @@ typedef diagnostic_event_id_t *diagnostic_event_id_ptr;
> /* A type for compactly referring to a particular thread within a
> diagnostic_path. Typically there is just one thread per path,
> with id 0. */
> -typedef unsigned diagnostic_thread_id_t;
> +typedef int diagnostic_thread_id_t;
>
> #endif /* ! GCC_DIAGNOSTIC_EVENT_ID_H */
> diff --git a/gcc/vec.h b/gcc/vec.h
> index bc83827f644..b13c4716428 100644
> --- a/gcc/vec.h
> +++ b/gcc/vec.h
> @@ -2409,7 +2409,7 @@ public:
> const value_type &back () const;
> const value_type &operator[] (unsigned int i) const;
>
> - size_t size () const { return m_size; }
> + unsigned size () const { return m_size; }
> size_t size_bytes () const { return m_size * sizeof (T); }
> bool empty () const { return m_size == 0; }
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 7dbae1d72e3..2f736a3b346 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -181,7 +181,7 @@ enum reg_class const regclass_map[FIRST_PSEUDO_REGISTER] =
>
> /* The "default" register map used in 32bit mode. */
>
> -int const debugger_register_map[FIRST_PSEUDO_REGISTER] =
> +unsigned int const debugger_register_map[FIRST_PSEUDO_REGISTER] =
> {
> /* general regs */
> 0, 2, 1, 3, 6, 7, 4, 5,
> @@ -212,7 +212,7 @@ int const debugger_register_map[FIRST_PSEUDO_REGISTER] =
>
> /* The "default" register map used in 64bit mode. */
>
> -int const debugger64_register_map[FIRST_PSEUDO_REGISTER] =
> +unsigned int const debugger64_register_map[FIRST_PSEUDO_REGISTER] =
> {
> /* general regs */
> 0, 1, 2, 3, 4, 5, 6, 7,
> @@ -294,7 +294,7 @@ int const debugger64_register_map[FIRST_PSEUDO_REGISTER] =
> 17 for %st(6) (gcc regno = 14)
> 18 for %st(7) (gcc regno = 15)
> */
> -int const svr4_debugger_register_map[FIRST_PSEUDO_REGISTER] =
> +unsigned int const svr4_debugger_register_map[FIRST_PSEUDO_REGISTER] =
> {
> /* general regs */
> 0, 2, 1, 3, 6, 7, 5, 4,
> diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> index 400a8856de2..19359662f8f 100644
> --- a/gcc/ipa-modref.cc
> +++ b/gcc/ipa-modref.cc
> @@ -1997,7 +1997,7 @@ struct escape_point
> /* Value escapes to this call. */
> gcall *call;
> /* Argument it escapes to. */
> - int arg;
> + unsigned int arg;
> /* Flags already known about the argument (this can save us from recording
> escape points if local analysis did good job already). */
> eaf_flags_t min_flags;
> @@ -2047,7 +2047,8 @@ public:
> bool merge_deref (const modref_lattice &with, bool ignore_stores);
> bool merge_direct_load ();
> bool merge_direct_store ();
> - bool add_escape_point (gcall *call, int arg, int min_flags, bool diret);
> + bool add_escape_point (gcall *call, unsigned int arg,
> + eaf_flags_t min_flags, bool direct);
> void dump (FILE *out, int indent = 0) const;
> };
>
> @@ -2101,8 +2102,8 @@ modref_lattice::dump (FILE *out, int indent) const
> point exists. */
>
> bool
> -modref_lattice::add_escape_point (gcall *call, int arg, int min_flags,
> - bool direct)
> +modref_lattice::add_escape_point (gcall *call, unsigned arg,
> + eaf_flags_t min_flags, bool direct)
> {
> escape_point *ep;
> unsigned int i;
> @@ -4415,12 +4416,12 @@ update_escape_summary_1 (cgraph_edge *e,
> continue;
> FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
> {
> - int min_flags = ee->min_flags;
> + eaf_flags_t min_flags = ee->min_flags;
> if (ee->direct && !em->direct)
> min_flags = deref_flags (min_flags, ignore_stores);
> struct escape_entry entry = {em->parm_index, ee->arg,
> min_flags,
> - ee->direct & em->direct};
> + ee->direct && em->direct};
> sum->esc.safe_push (entry);
> }
> }
> diff --git a/gcc/pair-fusion.cc b/gcc/pair-fusion.cc
> index cb0374f426b..653055fdcf6 100644
> --- a/gcc/pair-fusion.cc
> +++ b/gcc/pair-fusion.cc
> @@ -444,7 +444,7 @@ pair_fusion_bb_info::track_access (insn_info *insn, bool
> load_p, rtx mem)
> const bool fpsimd_op_p = m_pass->fpsimd_op_p (reg_op, mem_mode, load_p);
>
> // Note pair_operand_mode_ok_p already rejected VL modes.
> - const HOST_WIDE_INT mem_size = GET_MODE_SIZE (mem_mode).to_constant ();
> + const unsigned mem_size = GET_MODE_SIZE (mem_mode).to_constant ();
> const lfs_fields lfs = { load_p, fpsimd_op_p, mem_size };
>
> if (track_via_mem_expr (insn, mem, lfs))
> diff --git a/gcc/pretty-print.cc b/gcc/pretty-print.cc
> index 998e06e155f..0cd9b4d0a41 100644
> --- a/gcc/pretty-print.cc
> +++ b/gcc/pretty-print.cc
> @@ -1923,7 +1923,7 @@ format_phase_2 (pretty_printer *pp,
> /* When quoting, print alphanumeric, punctuation, and the space
> character unchanged, and all others in hexadecimal with the
> "\x" prefix. Otherwise print them all unchanged. */
> - int chr = va_arg (*text.m_args_ptr, int);
> + char chr = (char) va_arg (*text.m_args_ptr, int);
> if (ISPRINT (chr) || !quote)
> pp_character (pp, chr);
> else
> diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
> index 525eb357858..6552ddd1ee2 100644
> --- a/gcc/tree-ssa-loop-ch.cc
> +++ b/gcc/tree-ssa-loop-ch.cc
> @@ -839,8 +839,8 @@ ch_base::copy_headers (function *fun)
> copied. TODO -- handle while (a || b) - like cases, by not requiring
> the header to have just a single successor and copying up to
> postdominator. */
> - int nheaders = 0;
> - int last_win_nheaders = 0;
> + unsigned int nheaders = 0;
> + unsigned int last_win_nheaders = 0;
> bool last_win_invariant_exit = false;
> ch_decision ret;
> auto_vec <ch_decision, 32> decision;
> @@ -893,7 +893,7 @@ ch_base::copy_headers (function *fun)
> }
> /* "Duplicate" all BBs with zero cost following last basic blocks we
> decided to copy. */
> - while (last_win_nheaders < (int)decision.length ()
> + while (last_win_nheaders < decision.length ()
> && decision[last_win_nheaders] == ch_possible_zero_cost)
> {
> if (dump_file && (dump_flags & TDF_DETAILS))
> diff --git a/gcc/tree-ssa-structalias.cc b/gcc/tree-ssa-structalias.cc
> index a32ef1d5cc0..7adb50a896d 100644
> --- a/gcc/tree-ssa-structalias.cc
> +++ b/gcc/tree-ssa-structalias.cc
> @@ -5925,7 +5925,7 @@ field_must_have_pointers (tree t)
>
> static bool
> push_fields_onto_fieldstack (tree type, vec<fieldoff_s> *fieldstack,
> - HOST_WIDE_INT offset)
> + unsigned HOST_WIDE_INT offset)
> {
> tree field;
> bool empty_p = true;
> @@ -5943,7 +5943,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s>
> *fieldstack,
> if (TREE_CODE (field) == FIELD_DECL)
> {
> bool push = false;
> - HOST_WIDE_INT foff = bitpos_of_field (field);
> + unsigned HOST_WIDE_INT foff = bitpos_of_field (field);
Can you make bitpos_of_field return unsigned HOST_WIDE_INT then and adjust it
accordingly - it looks for shwi fitting but negative DECL_FIELD_OFFSET
or BIT_OFFSET
are not a thing.
> tree field_type = TREE_TYPE (field);
>
> if (!var_can_have_subvars (field)
> @@ -5988,7 +5988,7 @@ push_fields_onto_fieldstack (tree type, vec<fieldoff_s>
> *fieldstack,
> && !must_have_pointers_p
> && !pair->must_have_pointers
> && !pair->has_unknown_size
> - && pair->offset + (HOST_WIDE_INT)pair->size == offset + foff)
> + && pair->offset + pair->size == offset + foff)
> {
> pair->size += tree_to_uhwi (DECL_SIZE (field));
> }
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 9c817de18bd..0bd385f2328 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -7471,7 +7471,8 @@ vect_prologue_cost_for_slp (slp_tree node,
> nelt_limit = const_nunits;
> hash_set<vect_scalar_ops_slice_hash> vector_ops;
> for (unsigned int i = 0; i < SLP_TREE_NUMBER_OF_VEC_STMTS (node); ++i)
> - if (!vector_ops.add ({ ops, i * const_nunits, const_nunits }))
So why do we diagnose this case (unsigned int member) but not ...
> + if (!vector_ops.add
> + ({ ops, i * (unsigned)const_nunits, (unsigned)const_nunits }))
> starts.quick_push (i * const_nunits);
... this one - unsigned int function argument?
I think it would be slightly better to do
{
unsigned start = (unsigned) const_units * i;
if (!vector_ops.add ({ ops, start, const_unints }))
starts.quick_push (start);
}
to avoid the non-obvious difference between both.
OK with that change.
Richard.
> }
> else
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 45003f762dd..688b8715a15 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1711,11 +1711,11 @@ vect_truncate_gather_scatter_offset (stmt_vec_info
> stmt_info,
> count = max_iters.to_shwi ();
>
> /* Try scales of 1 and the element size. */
> - int scales[] = { 1, vect_get_scalar_dr_size (dr_info) };
> + unsigned int scales[] = { 1, vect_get_scalar_dr_size (dr_info) };
> wi::overflow_type overflow = wi::OVF_NONE;
> for (int i = 0; i < 2; ++i)
> {
> - int scale = scales[i];
> + unsigned int scale = scales[i];
> widest_int factor;
> if (!wi::multiple_of_p (wi::to_widest (step), scale, SIGNED, &factor))
> continue;
> @@ -6528,7 +6528,8 @@ vectorizable_operation (vec_info *vinfo,
> poly_uint64 nunits_in;
> poly_uint64 nunits_out;
> tree vectype_out;
> - int ncopies, vec_num;
> + unsigned int ncopies;
> + int vec_num;
> int i;
> vec<tree> vec_oprnds0 = vNULL;
> vec<tree> vec_oprnds1 = vNULL;
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 8a2d2b0438e..23f4884eff9 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -585,7 +585,6 @@ AC_SUBST(aliasing_flags)
> # * 'long long'
> # * variadic macros
> # * overlong strings
> -# * C++11 narrowing conversions in { }
> # So, we only use -pedantic if we can disable those warnings.
>
> # In stage 1, disable -Wformat warnings from old GCCs about new % codes
> @@ -595,7 +594,7 @@ AC_ARG_ENABLE(build-format-warnings,
> AS_IF([test $enable_build_format_warnings = no],
> [wf_opt=-Wno-format],[wf_opt=])
> ACX_PROG_CXX_WARNING_OPTS(
> - m4_quote(m4_do([-W -Wall -Wno-narrowing -Wwrite-strings ],
> + m4_quote(m4_do([-W -Wall -Wno-error=narrowing -Wwrite-strings ],
> [-Wcast-qual $wf_opt])),
> [loose_warn])
> ACX_PROG_CC_WARNING_OPTS(
> diff --git a/gcc/rtl-ssa/member-fns.inl b/gcc/rtl-ssa/member-fns.inl
> index d39184fb8cd..143c22c8c77 100644
> --- a/gcc/rtl-ssa/member-fns.inl
> +++ b/gcc/rtl-ssa/member-fns.inl
> @@ -41,7 +41,8 @@ access_array_builder::quick_push (access_info *access)
> inline array_slice<access_info *>
> access_array_builder::finish ()
> {
> - auto num_accesses = obstack_object_size (m_obstack) / sizeof (access_info
> *);
> + unsigned num_accesses
> + = obstack_object_size (m_obstack) / sizeof (access_info *);
> if (num_accesses == 0)
> return {};
>
> diff --git a/gcc/configure b/gcc/configure
> index 3d301b6ecd3..5acc42c1e4d 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -7146,7 +7146,6 @@ fi
> # * 'long long'
> # * variadic macros
> # * overlong strings
> -# * C++11 narrowing conversions in { }
> # So, we only use -pedantic if we can disable those warnings.
>
> # In stage 1, disable -Wformat warnings from old GCCs about new % codes
> @@ -7170,7 +7169,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
>
> loose_warn=
> save_CXXFLAGS="$CXXFLAGS"
> -for real_option in -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual
> $wf_opt; do
> +for real_option in -W -Wall -Wno-error=narrowing -Wwrite-strings -Wcast-qual
> $wf_opt; do
> # Do the check with the no- prefix removed since gcc silently
> # accepts any -Wno-* option on purpose
> case $real_option in
> @@ -21406,7 +21405,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 21409 "configure"
> +#line 21408 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -21512,7 +21511,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 21515 "configure"
> +#line 21514 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
>
> base-commit: 2484ba167e1c4a52d4989b71e1f5d4d27755500f
> --
> 2.46.0
>
