| Message ID | 20210602170926.GA7746@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | xtensa: Fix 2 warnings during xtensa build [PR100841] | expand |
On 6/2/2021 11:09 AM, Jakub Jelinek wrote: > Hi! > > 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 <jakub@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
On Wed, Jun 2, 2021 at 11:27 AM Jeff Law <jeffreyalaw@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 <jakub@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
--- gcc/config/xtensa/xtensa.h.jj 2021-01-04 10:25:45.570157539 +0100 +++ gcc/config/xtensa/xtensa.h 2021-06-02 17:41:04.190861829 +0200 @@ -279,7 +279,7 @@ extern const char xtensa_leaf_regs[FIRST /* For Xtensa, no remapping is necessary, but this macro must be defined if LEAF_REGISTERS is defined. */ -#define LEAF_REG_REMAP(REGNO) (REGNO) +#define LEAF_REG_REMAP(REGNO) ((int) (REGNO)) /* This must be declared if LEAF_REGISTERS is set. */ extern int leaf_function; @@ -775,8 +775,9 @@ typedef struct xtensa_args #define INCOMING_RETURN_ADDR_RTX gen_rtx_REG (Pmode, 0) #define DWARF_FRAME_RETURN_COLUMN DWARF_FRAME_REGNUM (0) #define DWARF_ALT_FRAME_RETURN_COLUMN 16 -#define DWARF_FRAME_REGISTERS (DWARF_ALT_FRAME_RETURN_COLUMN \ - + (TARGET_WINDOWED_ABI ? 0 : 1)) +#define DWARF_FRAME_REGISTERS (TARGET_WINDOWED_ABI \ + ? DWARF_ALT_FRAME_RETURN_COLUMN \ + : DWARF_ALT_FRAME_RETURN_COLUMN + 1) #define EH_RETURN_DATA_REGNO(N) ((N) < 2 ? (N) + 2 : INVALID_REGNUM) #define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \ (flag_pic \