On Thu, 2022-05-19 at 15:16 +0200, Bernhard Reutner-Fischer via Gcc-
patches wrote:
> Hi Andrew,
>
> On Tue, 17 May 2022 14:38:39 -0400
> Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> > commit b7501739f3b14ac7749aace93f636d021fd607f7
> > Author: Andrew MacLeod <amacl...@redhat.com>
> > Date: Mon May 9 15:35:14 2022 -0400
>
> > diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> > index d3cf8be9bd8..56f4577cfbb 100644
> > --- a/gcc/gimple-range-cache.cc
> > +++ b/gcc/gimple-range-cache.cc
> > @@ -1545,81 +1426,42 @@ ranger_cache::range_from_dom (irange &r,
> > tree name, basic_block start_bb,
>
> > void
> > -ranger_cache::update_to_nonnull (basic_block bb, tree name)
> > +ranger_cache::apply_side_effects (gimple *s)
> > {
>
> > - if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) ==
> > TARGET_MEM_REF)
> > + // Do not update the on-netry cache for block ending stmts.
>
> s/on-netry/on-entry/
>
> > diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-
> > side-effect.cc
> > new file mode 100644
> > index 00000000000..2c8c77dc569
> > --- /dev/null
> > +++ b/gcc/gimple-range-side-effect.cc
>
> > +// ---------------------------------------------------------------
> > ----------
> > +
> > +// This class is an element in list of side effect ranges.
>
> s/in list/in the list/
>
> > +// Construct a side effects manager. DO_SEARCH indicates whether
> > an immediate
> > +// use scan should be made the first time a name is processed.
> > This is for
> > +// on-demand clients who may not visit every statement and may
> > miss uses.
>
> may otherwise miss uses, i suppose. But ok.
>
> > diff --git a/gcc/gimple-range-side-effect.h b/gcc/gimple-range-
> > side-effect.h
> > new file mode 100644
> > index 00000000000..848d94ba6d7
> > --- /dev/null
> > +++ b/gcc/gimple-range-side-effect.h
> > @@ -0,0 +1,82 @@
>
> > +// New side effects should added in the constructor of this class.
>
> s/should added/should be added/
>
> > +
> > +class stmt_side_effects
> > +{
>
> > +};
> > +
> > +// This class manages a list of side effect ranges for each basic
> > block.
> > +// As side effects are seen, they can be registered to a block and
> > later
> > +// queried. WHen constructed with a TRUE flag, immediate uses
> > chains are
> > +// followed the first time a name is referenced and block
> > populated if
> > +// thre are any side effects.
>
> s/WHen/When/
> s/thre are/there are/
>
> > +
> > +class side_effect_manager
> > +{
>
> > diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > index 1fdee026a4b..f5e9e77bc71 100644
> > --- a/gcc/gimple-range.cc
> > +++ b/gcc/gimple-range.cc
>
> > @@ -118,9 +118,11 @@ gimple_ranger::range_of_expr (irange &r, tree
> > expr, gimple *stmt)
> > // If name is defined in this block, try to get an range
> > from S.
> > if (def_stmt && gimple_bb (def_stmt) == bb)
> > {
> > - // Check for a definition override from a block walk.
> > - if (!POINTER_TYPE_P (TREE_TYPE (expr))
> > - || !m_cache.block_range (r, bb, expr, false))
> > + // Declared in ths block, if it has a global set, check
> > for an
> > + // override from a block walk, otherwise calculate it.
>
> s/ths/this/
>
> And, btw, i see
> In file included from ../../../src/gcc-13.mine/gcc/tree-vrp.h:23,
> from ../../../src/gcc-13.mine/gcc/ssa.h:28,
> from ../../../src/gcc-13.mine/gcc/gimple-warn-
> types.cc:30:
> ../../../src/gcc-13.mine/gcc/value-range.h: In member function
> ‘unsigned int irange::num_pairs() const’:
> ../../../src/gcc-13.mine/gcc/value-range.h:217:22: warning: Function
> ‘num_pairs’ could return ‘unsigned char’ [--all-warnings]
> 217 | irange::num_pairs () const
> | ^~~~~
> | unsigned char
> ../../../src/gcc-13.mine/gcc/value-range.h:217:22: note: with a range
> of [0,255]
The location for that warning does look wrong, but this doesn't quite
look like one of our warnings - was this from GCC? Which version?
Otherwise, which compiler emitted it?
>
> The C++ member function decl result locations are all wrong, i know,
> but i think the warning is correct. We've got:
> class GTY((user)) irange
> {
> unsigned char m_num_ranges;
> }
> but
> inline unsigned
> irange::num_pairs () const
> {
> if (m_kind == VR_ANTI_RANGE)
> return constant_p () ? 2 : 1;
> else
> return m_num_ranges;
> }
I don't see what's wrong with the code. It's OK to promote an unsigned
char to an unsigned int, isn't it? The compiler is required to
preserve the value.
Sorry if I'm missing something here
Dave
