On Wed, Jun 2, 2021 at 11:27 AM Jeff Law <jeffreya...@gmail.com> wrote:
> On 6/2/2021 11:09 AM, Jakub Jelinek wrote:
> > When building gcc targetting xtensa-linux, there are 2 warnings the PR
> > complains about:
> > ../../gcc/dwarf2cfi.c: In function ‘void init_one_dwarf_reg_size(int, 
> > machine_mode, rtx, machine_mode, init_one_dwarf_reg_state*)’:
> > ../../gcc/dwarf2cfi.c:291:12: warning: comparison of integer expressions of 
> > different signedness: ‘const unsigned int’ and ‘int’ [-Wsign-compare]
> >    291 |   if (rnum >= DWARF_FRAME_REGISTERS)
> > ../../gcc/function.c: In function ‘void gen_call_used_regs_seq(rtx_insn*, 
> > unsigned int)’:
> > ../../gcc/function.c:5897:63: warning: comparison of unsigned expression in 
> > ‘< 0’ is always false [-Wtype-limits]
> >   5897 |       if (crtl->uses_only_leaf_regs && LEAF_REG_REMAP (regno) < 0)
> > which might during bootstrap or when configured with --enable-werror-always
> > be turned into errors.
> >
> > The first one is the -Wsign-compare warning, in c-family we do:
> > 2281        /* Do not warn if the signed quantity is an unsuffixed integer
> > 2282           literal (or some static constant expression involving such
> > 2283           literals or a conditional expression involving such literals)
> > 2284           and it is non-negative.  */
> > 2285        if (tree_expr_nonnegative_warnv_p (sop, &ovf))
> > 2286          /* OK */;
> > and so don't warn if that function determines the expression is
> > non-negative.  But xtensa defines DWARF_FRAME_REGISTERS as
> > (16 + (something ? 0 : 1)) and that isn't handled by
> > tree_expr_nonnegative_warnv_p, VRP can handle it of course, but that is much
> > later.
> > The second chunk rewrites it into a form that tree_expr_nonnegative_warnv_p
> > can handle, in particular (something ? 16 : 16 + 1), where for COND_EXPRs
> > that function checks both the 2nd and 3rd operand of the ternary operator
> > and if both are nonnegative, returns true.
> >
> > The other warning has been introduced fairly recently; LEAF_REG_REMAP is
> > currently used by 2 targets only, and is documented to yield -1 if a hard
> > reg number can't be remapped and the remapped register number otherwise.
> > That means that the type of the expression should be signed (otherwise -1
> > could never appear), and on SPARC indeed it is defined as
> > extern char leaf_reg_remap[];
> > #define LEAF_REG_REMAP(REGNO) (leaf_reg_remap[REGNO])
> > so unless the host is -funsigned-char by default it works fine.
> > I guess sparc.[ch] should be fixed to use signed char of leaf_reg_remap,
> > Eric?
> > The argument to LEAF_REG_REMAP is often unsigned int though, hard
> > register numbers are usually not negative, and thus the warning.
> > I think xtensa doesn't have 2G hard registers and so it is ok to just cast
> > it to int.
> >
> > Verified just by making sure the warnings go away in a cross, ok for trunk?
> >
> > 2021-06-02  Jakub Jelinek  <ja...@redhat.com>
> >
> >       PR target/100841
> >       * config/xtensa/xtensa.h (LEAF_REG_REMAP): Cast REGNO to int to avoid
> >       -Wtype-limits warnings.
> >       (DWARF_FRAME_REGISTER): Rewrite into ternary operator with addition
> >       in operands to avoid -Wsign-compare warnings.
> OK.
> jeff

Looks good to me. Thank you!

-- Max

Reply via email to