| Message ID | 8738b9leiv.fsf@e105548-lin.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
Richard Sandiford <richard.sandiford@arm.com> writes: > @@ -315,7 +318,7 @@ struct ira_allocno > number (0, ...) - 2. Value -1 is used for allocnos spilled by the > reload (at this point pseudo-register has only one allocno) which > did not get stack slot yet. */ > - short int hard_regno; > + int hard_regno : 16; If you want negative numbers you need to make that explicitly signed. Andreas.
Andreas Schwab <schwab@suse.de> writes: > Richard Sandiford <richard.sandiford@arm.com> writes: > >> @@ -315,7 +318,7 @@ struct ira_allocno >> number (0, ...) - 2. Value -1 is used for allocnos spilled by the >> reload (at this point pseudo-register has only one allocno) which >> did not get stack slot yet. */ >> - short int hard_regno; >> + int hard_regno : 16; > > If you want negative numbers you need to make that explicitly signed. Are you sure? In: struct { int i : 16; unsigned int j : 1; } x = { -1, 0 }; int foo (void) { return x.i; } foo returns -1 rather than 65535. I can't see any precedent in gcc/*.[hc] for explicitly marking bitfields as signed. Thanks, Richard
Richard Sandiford <richard.sandiford@arm.com> writes: > Andreas Schwab <schwab@suse.de> writes: >> Richard Sandiford <richard.sandiford@arm.com> writes: >> >>> @@ -315,7 +318,7 @@ struct ira_allocno >>> number (0, ...) - 2. Value -1 is used for allocnos spilled by the >>> reload (at this point pseudo-register has only one allocno) which >>> did not get stack slot yet. */ >>> - short int hard_regno; >>> + int hard_regno : 16; >> >> If you want negative numbers you need to make that explicitly signed. > > Are you sure? See C11, 6.7.2#5. Each of the comma-separated multisets designates the same type, except that for bit-fields, it is implementation-defined whether the specifier int designates the same type as signed int or the same type as unsigned int. Andreas.
On 30/09/14 12:51, Andreas Schwab wrote: > Richard Sandiford <richard.sandiford@arm.com> writes: > >> Andreas Schwab <schwab@suse.de> writes: >>> Richard Sandiford <richard.sandiford@arm.com> writes: >>> >>>> @@ -315,7 +318,7 @@ struct ira_allocno >>>> number (0, ...) - 2. Value -1 is used for allocnos spilled by the >>>> reload (at this point pseudo-register has only one allocno) which >>>> did not get stack slot yet. */ >>>> - short int hard_regno; >>>> + int hard_regno : 16; >>> >>> If you want negative numbers you need to make that explicitly signed. >> >> Are you sure? > > See C11, 6.7.2#5. > > Each of the comma-separated multisets designates the same type, > except that for bit-fields, it is implementation-defined whether the > specifier int designates the same type as signed int or the same > type as unsigned int. > > > Andreas. > GCC is written in C++ these days, so technically, you need the C++ standard :-) GNU C defaults to signed bitfields (see trouble.texi). However, since GCC is supposed to bootstrap using a portable ISO C++ compiler, there's an argument for removing the ambiguity entirely by being explicit. We no-longer have to worry about compilers that don't support the signed keyword. R.
On Tue, 30 Sep 2014, Richard Earnshaw wrote: > GCC is written in C++ these days, so technically, you need the C++ > standard :-) And, while C++14 requires plain int bit-fields to be signed, GCC is written in C++98/C++03.
On Sep 30, 2014, at 9:15 AM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Tue, 30 Sep 2014, Richard Earnshaw wrote: > >> GCC is written in C++ these days, so technically, you need the C++ >> standard :-) > > And, while C++14 requires plain int bit-fields to be signed, GCC is > written in C++98/C++03. So, seemingly left unstated in the thread is what is required by the language standard we write in… From c++98: It is implementa- tion-defined whether bit-fields and objects of char type are repre- sented as signed or unsigned quantities. The signed specifier forces char objects and bit-fields to be signed; it is redundant with other integral types. So, I think you need a signed on bitfields if your want them to be signed. It doesn’t matter what g++ does, if we want to be portable to any C++ compiler.
"Joseph S. Myers" <joseph@codesourcery.com> writes:
> And, while C++14 requires plain int bit-fields to be signed,
Thanks, noted for the future.
Andreas.
Hi Andreas, OK, I will fix this. Thanks, David Sherwood. -----Original Message----- From: Andreas Schwab [mailto:schwab@suse.de] Sent: 01 October 2014 08:27 To: Joseph S. Myers Cc: Richard Earnshaw; David Sherwood; gcc-patches@gcc.gnu.org; vmakarov@redhat.com; Richard Sandiford Subject: Re: Fix for "FAIL: tmpdir-gcc.dg-struct-layout-1/t028 c_compat_x_tst.o compile, (internal compiler error)" "Joseph S. Myers" <joseph@codesourcery.com> writes: > And, while C++14 requires plain int bit-fields to be signed, Thanks, noted for the future. Andreas.
On 30/09/14 20:33, Mike Stump wrote: > On Sep 30, 2014, at 9:15 AM, Joseph S. Myers <joseph@codesourcery.com> wrote: >> On Tue, 30 Sep 2014, Richard Earnshaw wrote: >> >>> GCC is written in C++ these days, so technically, you need the C++ >>> standard :-) >> >> And, while C++14 requires plain int bit-fields to be signed, GCC is >> written in C++98/C++03. > > So, seemingly left unstated in the thread is what is required by the language standard we write in… From c++98: > Isn't that exactly what I suggested? "However, since GCC is supposed to bootstrap using a portable ISO C++ compiler, there's an argument for removing the ambiguity entirely by being explicit." > It is implementa- > tion-defined whether bit-fields and objects of char type are repre- > sented as signed or unsigned quantities. The signed specifier forces > char objects and bit-fields to be signed; it is redundant with other > integral types. > > So, I think you need a signed on bitfields if your want them to be signed. It doesn’t matter what g++ does, if we want to be portable to any C++ compiler. > R.
On Oct 1, 2014, at 1:50 AM, Richard Earnshaw <rearnsha@arm.com> wrote: > Isn't that exactly what I suggested? > > "However, since > GCC is supposed to bootstrap using a portable ISO C++ compiler, there's > an argument for removing the ambiguity entirely by being explicit.” I think one can read that and not understand that in the language standard, it is a requirement. While those that know C++ well, know that, for those that don’t, I think it helps to see why it is a requirement.
Index: gcc/ira-int.h =================================================================== --- gcc/ira-int.h 2014-09-22 08:36:23.613797736 +0100 +++ gcc/ira-int.h 2014-09-30 08:50:55.936083472 +0100 @@ -283,6 +283,9 @@ struct ira_allocno /* Mode of the allocno which is the mode of the corresponding pseudo-register. */ ENUM_BITFIELD (machine_mode) mode : 8; + /* Widest mode of the allocno which in at least one case could be + for paradoxical subregs where wmode > mode. */ + ENUM_BITFIELD (machine_mode) wmode : 8; /* Register class which should be used for allocation for given allocno. NO_REGS means that we should use memory. */ ENUM_BITFIELD (reg_class) aclass : 16; @@ -315,7 +318,7 @@ struct ira_allocno number (0, ...) - 2. Value -1 is used for allocnos spilled by the reload (at this point pseudo-register has only one allocno) which did not get stack slot yet. */ - short int hard_regno; + int hard_regno : 16; /* Allocnos with the same regno are linked by the following member. Allocnos corresponding to inner loops are first in the list (it corresponds to depth-first traverse of the loops). */ @@ -436,6 +439,7 @@ #define ALLOCNO_TOTAL_NO_STACK_REG_P(A) #define ALLOCNO_BAD_SPILL_P(A) ((A)->bad_spill_p) #define ALLOCNO_ASSIGNED_P(A) ((A)->assigned_p) #define ALLOCNO_MODE(A) ((A)->mode) +#define ALLOCNO_WMODE(A) ((A)->wmode) #define ALLOCNO_PREFS(A) ((A)->allocno_prefs) #define ALLOCNO_COPIES(A) ((A)->allocno_copies) #define ALLOCNO_HARD_REG_COSTS(A) ((A)->hard_reg_costs) Index: gcc/ira-build.c =================================================================== --- gcc/ira-build.c 2014-08-26 12:09:28.234659250 +0100 +++ gcc/ira-build.c 2014-09-30 09:00:05.541337392 +0100 @@ -524,6 +524,7 @@ ira_create_allocno (int regno, bool cap_ ALLOCNO_BAD_SPILL_P (a) = false; ALLOCNO_ASSIGNED_P (a) = false; ALLOCNO_MODE (a) = (regno < 0 ? VOIDmode : PSEUDO_REGNO_MODE (regno)); + ALLOCNO_WMODE (a) = ALLOCNO_MODE (a); ALLOCNO_PREFS (a) = NULL; ALLOCNO_COPIES (a) = NULL; ALLOCNO_HARD_REG_COSTS (a) = NULL; @@ -893,6 +894,7 @@ create_cap_allocno (ira_allocno_t a) parent = ALLOCNO_LOOP_TREE_NODE (a)->parent; cap = ira_create_allocno (ALLOCNO_REGNO (a), true, parent); ALLOCNO_MODE (cap) = ALLOCNO_MODE (a); + ALLOCNO_WMODE (cap) = ALLOCNO_WMODE (a); aclass = ALLOCNO_CLASS (a); ira_set_allocno_class (cap, aclass); ira_create_allocno_objects (cap); @@ -1859,9 +1861,9 @@ ira_traverse_loop_tree (bool bb_p, ira_l /* This recursive function creates allocnos corresponding to pseudo-registers containing in X. True OUTPUT_P means that X is - a lvalue. */ + an lvalue. PARENT corresponds to the parent expression of X. */ static void -create_insn_allocnos (rtx x, bool output_p) +create_insn_allocnos (rtx x, rtx outer, bool output_p) { int i, j; const char *fmt; @@ -1876,7 +1878,15 @@ create_insn_allocnos (rtx x, bool output ira_allocno_t a; if ((a = ira_curr_regno_allocno_map[regno]) == NULL) - a = ira_create_allocno (regno, false, ira_curr_loop_tree_node); + { + a = ira_create_allocno (regno, false, ira_curr_loop_tree_node); + if (outer != NULL && GET_CODE (outer) == SUBREG) + { + enum machine_mode wmode = GET_MODE (outer); + if (GET_MODE_SIZE (wmode) > GET_MODE_SIZE (ALLOCNO_WMODE (a))) + ALLOCNO_WMODE (a) = wmode; + } + } ALLOCNO_NREFS (a)++; ALLOCNO_FREQ (a) += REG_FREQ_FROM_BB (curr_bb); @@ -1887,25 +1897,25 @@ create_insn_allocnos (rtx x, bool output } else if (code == SET) { - create_insn_allocnos (SET_DEST (x), true); - create_insn_allocnos (SET_SRC (x), false); + create_insn_allocnos (SET_DEST (x), NULL, true); + create_insn_allocnos (SET_SRC (x), NULL, false); return; } else if (code == CLOBBER) { - create_insn_allocnos (XEXP (x, 0), true); + create_insn_allocnos (XEXP (x, 0), NULL, true); return; } else if (code == MEM) { - create_insn_allocnos (XEXP (x, 0), false); + create_insn_allocnos (XEXP (x, 0), NULL, false); return; } else if (code == PRE_DEC || code == POST_DEC || code == PRE_INC || code == POST_INC || code == POST_MODIFY || code == PRE_MODIFY) { - create_insn_allocnos (XEXP (x, 0), true); - create_insn_allocnos (XEXP (x, 0), false); + create_insn_allocnos (XEXP (x, 0), NULL, true); + create_insn_allocnos (XEXP (x, 0), NULL, false); return; } @@ -1913,10 +1923,10 @@ create_insn_allocnos (rtx x, bool output for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--) { if (fmt[i] == 'e') - create_insn_allocnos (XEXP (x, i), output_p); + create_insn_allocnos (XEXP (x, i), x, output_p); else if (fmt[i] == 'E') for (j = 0; j < XVECLEN (x, i); j++) - create_insn_allocnos (XVECEXP (x, i, j), output_p); + create_insn_allocnos (XVECEXP (x, i, j), x, output_p); } } @@ -1935,7 +1945,7 @@ create_bb_allocnos (ira_loop_tree_node_t ira_assert (bb != NULL); FOR_BB_INSNS_REVERSE (bb, insn) if (NONDEBUG_INSN_P (insn)) - create_insn_allocnos (PATTERN (insn), false); + create_insn_allocnos (PATTERN (insn), NULL, false); /* It might be a allocno living through from one subloop to another. */ EXECUTE_IF_SET_IN_REG_SET (df_get_live_in (bb), FIRST_PSEUDO_REGISTER, i, bi) Index: gcc/ira-conflicts.c =================================================================== --- gcc/ira-conflicts.c 2014-09-22 08:36:22.597810549 +0100 +++ gcc/ira-conflicts.c 2014-09-30 09:00:34.048986621 +0100 @@ -774,6 +774,27 @@ ira_build_conflicts (void) temp_hard_reg_set); } + /* Now we deal with paradoxical subreg cases where certain registers + cannot be accessed in the widest mode. */ + enum machine_mode outer_mode = ALLOCNO_WMODE (a); + enum machine_mode inner_mode = ALLOCNO_MODE (a); + if (GET_MODE_SIZE (outer_mode) > GET_MODE_SIZE (inner_mode)) + { + enum reg_class aclass = ALLOCNO_CLASS (a); + for (int j = ira_class_hard_regs_num[aclass] - 1; j >= 0; --j) + { + int inner_regno = ira_class_hard_regs[aclass][j]; + int outer_regno = simplify_subreg_regno (inner_regno, + inner_mode, 0, + outer_mode); + if (outer_regno < 0 + || !in_hard_reg_set_p (reg_class_contents[aclass], + outer_mode, outer_regno)) + SET_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS (obj), + inner_regno); + } + } + if (ALLOCNO_CALLS_CROSSED_NUM (a) != 0) { int regno;