| Message ID | 20130917081856.GB60115@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
Ping 2013/9/17 Ilya Enkovich <enkovich.gnu@gmail.com>: > Hi, > > Here is a patch introducing new type and mode for bounds. It is a part of MPX ISA support patch (http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01094.html). > > Bootstrapped and tested on linux-x86_64. Is it OK for trunk? > > Thanks, > Ilya > -- > > gcc/ > > 2013-09-16 Ilya Enkovich <ilya.enkovich@intel.com> > > * mode-classes.def (MODE_BOUND): New. > * tree.def (BOUND_TYPE): New. > * genmodes.c (complete_mode): Support MODE_BOUND. > (BOUND_MODE): New. > (make_bound_mode): New. > * machmode.h (BOUND_MODE_P): New. > * stor-layout.c (int_mode_for_mode): Support MODE_BOUND. > (layout_type): Support BOUND_TYPE. > * tree-pretty-print.c (dump_generic_node): Support BOUND_TYPE. > * tree.c (build_int_cst_wide): Support BOUND_TYPE. > (type_contains_placeholder_1): Likewise. > * tree.h (BOUND_TYPE_P): New. > * varasm.c (output_constant): Support BOUND_TYPE. > * doc/rtl.texi (MODE_BOUND): New. > > diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi > index 1d62223..02b1214 100644 > --- a/gcc/doc/rtl.texi > +++ b/gcc/doc/rtl.texi > @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the @file{@var{machine}-modes.def}. > @xref{Jump Patterns}, > also see @ref{Condition Code}. > > +@findex MODE_BOUND > +@item MODE_BOUND > +Bound modes class. Used to represent values of pointer bounds. > + > @findex MODE_RANDOM > @item MODE_RANDOM > This is a catchall mode class for modes which don't fit into the above > diff --git a/gcc/genmodes.c b/gcc/genmodes.c > index dc38483..89174ec 100644 > --- a/gcc/genmodes.c > +++ b/gcc/genmodes.c > @@ -333,6 +333,7 @@ complete_mode (struct mode_data *m) > break; > > case MODE_INT: > + case MODE_BOUND: > case MODE_FLOAT: > case MODE_DECIMAL_FLOAT: > case MODE_FRACT: > @@ -533,6 +534,18 @@ make_special_mode (enum mode_class cl, const char *name, > new_mode (cl, name, file, line); > } > > +#define BOUND_MODE(N, Y) make_bound_mode (#N, Y, __FILE__, __LINE__) > + > +static void ATTRIBUTE_UNUSED > +make_bound_mode (const char *name, > + unsigned int bytesize, > + const char *file, unsigned int line) > +{ > + struct mode_data *m = new_mode (MODE_BOUND, name, file, line); > + m->bytesize = bytesize; > +} > + > + > #define INT_MODE(N, Y) FRACTIONAL_INT_MODE (N, -1U, Y) > #define FRACTIONAL_INT_MODE(N, B, Y) \ > make_int_mode (#N, B, Y, __FILE__, __LINE__) > diff --git a/gcc/machmode.h b/gcc/machmode.h > index 981ee92..d4a20b2 100644 > --- a/gcc/machmode.h > +++ b/gcc/machmode.h > @@ -174,6 +174,9 @@ extern const unsigned char mode_class[NUM_MACHINE_MODES]; > || CLASS == MODE_ACCUM \ > || CLASS == MODE_UACCUM) > > +#define BOUND_MODE_P(MODE) \ > + (GET_MODE_CLASS (MODE) == MODE_BOUND) > + > /* Get the size in bytes and bits of an object of mode MODE. */ > > extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES]; > diff --git a/gcc/mode-classes.def b/gcc/mode-classes.def > index 7207ef7..c5ea215 100644 > --- a/gcc/mode-classes.def > +++ b/gcc/mode-classes.def > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see > DEF_MODE_CLASS (MODE_RANDOM), /* other */ \ > DEF_MODE_CLASS (MODE_CC), /* condition code in a register */ \ > DEF_MODE_CLASS (MODE_INT), /* integer */ \ > + DEF_MODE_CLASS (MODE_BOUND), /* bounds */ \ > DEF_MODE_CLASS (MODE_PARTIAL_INT), /* integer with padding bits */ \ > DEF_MODE_CLASS (MODE_FRACT), /* signed fractional number */ \ > DEF_MODE_CLASS (MODE_UFRACT), /* unsigned fractional number */ \ > diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c > index 6f6b310..82611c7 100644 > --- a/gcc/stor-layout.c > +++ b/gcc/stor-layout.c > @@ -383,6 +383,7 @@ int_mode_for_mode (enum machine_mode mode) > case MODE_VECTOR_ACCUM: > case MODE_VECTOR_UFRACT: > case MODE_VECTOR_UACCUM: > + case MODE_BOUND: > mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0); > break; > > @@ -2135,6 +2136,13 @@ layout_type (tree type) > SET_TYPE_MODE (type, VOIDmode); > break; > > + case BOUND_TYPE: > + SET_TYPE_MODE (type, > + mode_for_size (TYPE_PRECISION (type), MODE_BOUND, 0)); > + TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type))); > + TYPE_SIZE_UNIT (type) = size_int (GET_MODE_SIZE (TYPE_MODE (type))); > + break; > + > case OFFSET_TYPE: > TYPE_SIZE (type) = bitsize_int (POINTER_SIZE); > TYPE_SIZE_UNIT (type) = size_int (POINTER_SIZE / BITS_PER_UNIT); > diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c > index 69e4006..8b0825c 100644 > --- a/gcc/tree-pretty-print.c > +++ b/gcc/tree-pretty-print.c > @@ -697,6 +697,7 @@ dump_generic_node (pretty_printer *buffer, tree node, int spc, int flags, > break; > > case VOID_TYPE: > + case BOUND_TYPE: > case INTEGER_TYPE: > case REAL_TYPE: > case FIXED_POINT_TYPE: > diff --git a/gcc/tree.c b/gcc/tree.c > index b469b97..bbbe16e 100644 > --- a/gcc/tree.c > +++ b/gcc/tree.c > @@ -1197,6 +1197,7 @@ build_int_cst_wide (tree type, unsigned HOST_WIDE_INT low, HOST_WIDE_INT hi) > > case INTEGER_TYPE: > case OFFSET_TYPE: > + case BOUND_TYPE: > if (TYPE_UNSIGNED (type)) > { > /* Cache 0..N */ > @@ -3232,6 +3233,7 @@ type_contains_placeholder_1 (const_tree type) > switch (TREE_CODE (type)) > { > case VOID_TYPE: > + case BOUND_TYPE: > case COMPLEX_TYPE: > case ENUMERAL_TYPE: > case BOOLEAN_TYPE: > diff --git a/gcc/tree.def b/gcc/tree.def > index f825aad..b01cdd5 100644 > --- a/gcc/tree.def > +++ b/gcc/tree.def > @@ -232,6 +232,11 @@ DEFTREECODE (QUAL_UNION_TYPE, "qual_union_type", tcc_type, 0) > /* The void type in C */ > DEFTREECODE (VOID_TYPE, "void_type", tcc_type, 0) > > +/* Type to hold bounds for a pointer. > + Has TYPE_PRECISION component to specify number of bits used > + by this type. */ > +DEFTREECODE (BOUND_TYPE, "bound_type", tcc_type, 0) > + > /* Type of functions. Special fields: > TREE_TYPE type of value returned. > TYPE_ARG_TYPES list of types of arguments expected. > diff --git a/gcc/tree.h b/gcc/tree.h > index 83edaba..863a204 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -1097,6 +1097,10 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, > /* Nonzero if this type is a complete type. */ > #define COMPLETE_TYPE_P(NODE) (TYPE_SIZE (NODE) != NULL_TREE) > > +/* Nonzero if this type is a bound type. */ > +#define BOUND_TYPE_P(NODE) \ > + (TREE_CODE (NODE) == BOUND_TYPE) > + > /* Nonzero if this type is the (possibly qualified) void type. */ > #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE) > > diff --git a/gcc/varasm.c b/gcc/varasm.c > index 0504eeb..2b5305b 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -4703,6 +4703,7 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align) > case REFERENCE_TYPE: > case OFFSET_TYPE: > case FIXED_POINT_TYPE: > + case BOUND_TYPE: > if (! assemble_integer (expand_expr (exp, NULL_RTX, VOIDmode, > EXPAND_INITIALIZER), > MIN (size, thissize), align, 0))
Hey guys, could please someone look at this small patch? It blocks approved MPX ISA support on i386 target. Thanks, Ilya 2013/10/2 Ilya Enkovich <enkovich.gnu@gmail.com>: > Ping > > 2013/9/17 Ilya Enkovich <enkovich.gnu@gmail.com>: >> Hi, >> >> Here is a patch introducing new type and mode for bounds. It is a part of MPX ISA support patch (http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01094.html). >> >> Bootstrapped and tested on linux-x86_64. Is it OK for trunk? >> >> Thanks, >> Ilya >> -- >> >> gcc/ >> >> 2013-09-16 Ilya Enkovich <ilya.enkovich@intel.com> >> >> * mode-classes.def (MODE_BOUND): New. >> * tree.def (BOUND_TYPE): New. >> * genmodes.c (complete_mode): Support MODE_BOUND. >> (BOUND_MODE): New. >> (make_bound_mode): New. >> * machmode.h (BOUND_MODE_P): New. >> * stor-layout.c (int_mode_for_mode): Support MODE_BOUND. >> (layout_type): Support BOUND_TYPE. >> * tree-pretty-print.c (dump_generic_node): Support BOUND_TYPE. >> * tree.c (build_int_cst_wide): Support BOUND_TYPE. >> (type_contains_placeholder_1): Likewise. >> * tree.h (BOUND_TYPE_P): New. >> * varasm.c (output_constant): Support BOUND_TYPE. >> * doc/rtl.texi (MODE_BOUND): New. >> >> diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi >> index 1d62223..02b1214 100644 >> --- a/gcc/doc/rtl.texi >> +++ b/gcc/doc/rtl.texi >> @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the @file{@var{machine}-modes.def}. >> @xref{Jump Patterns}, >> also see @ref{Condition Code}. >> >> +@findex MODE_BOUND >> +@item MODE_BOUND >> +Bound modes class. Used to represent values of pointer bounds. >> + >> @findex MODE_RANDOM >> @item MODE_RANDOM >> This is a catchall mode class for modes which don't fit into the above >> diff --git a/gcc/genmodes.c b/gcc/genmodes.c >> index dc38483..89174ec 100644 >> --- a/gcc/genmodes.c >> +++ b/gcc/genmodes.c >> @@ -333,6 +333,7 @@ complete_mode (struct mode_data *m) >> break; >> >> case MODE_INT: >> + case MODE_BOUND: >> case MODE_FLOAT: >> case MODE_DECIMAL_FLOAT: >> case MODE_FRACT: >> @@ -533,6 +534,18 @@ make_special_mode (enum mode_class cl, const char *name, >> new_mode (cl, name, file, line); >> } >> >> +#define BOUND_MODE(N, Y) make_bound_mode (#N, Y, __FILE__, __LINE__) >> + >> +static void ATTRIBUTE_UNUSED >> +make_bound_mode (const char *name, >> + unsigned int bytesize, >> + const char *file, unsigned int line) >> +{ >> + struct mode_data *m = new_mode (MODE_BOUND, name, file, line); >> + m->bytesize = bytesize; >> +} >> + >> + >> #define INT_MODE(N, Y) FRACTIONAL_INT_MODE (N, -1U, Y) >> #define FRACTIONAL_INT_MODE(N, B, Y) \ >> make_int_mode (#N, B, Y, __FILE__, __LINE__) >> diff --git a/gcc/machmode.h b/gcc/machmode.h >> index 981ee92..d4a20b2 100644 >> --- a/gcc/machmode.h >> +++ b/gcc/machmode.h >> @@ -174,6 +174,9 @@ extern const unsigned char mode_class[NUM_MACHINE_MODES]; >> || CLASS == MODE_ACCUM \ >> || CLASS == MODE_UACCUM) >> >> +#define BOUND_MODE_P(MODE) \ >> + (GET_MODE_CLASS (MODE) == MODE_BOUND) >> + >> /* Get the size in bytes and bits of an object of mode MODE. */ >> >> extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES]; >> diff --git a/gcc/mode-classes.def b/gcc/mode-classes.def >> index 7207ef7..c5ea215 100644 >> --- a/gcc/mode-classes.def >> +++ b/gcc/mode-classes.def >> @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see >> DEF_MODE_CLASS (MODE_RANDOM), /* other */ \ >> DEF_MODE_CLASS (MODE_CC), /* condition code in a register */ \ >> DEF_MODE_CLASS (MODE_INT), /* integer */ \ >> + DEF_MODE_CLASS (MODE_BOUND), /* bounds */ \ >> DEF_MODE_CLASS (MODE_PARTIAL_INT), /* integer with padding bits */ \ >> DEF_MODE_CLASS (MODE_FRACT), /* signed fractional number */ \ >> DEF_MODE_CLASS (MODE_UFRACT), /* unsigned fractional number */ \ >> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c >> index 6f6b310..82611c7 100644 >> --- a/gcc/stor-layout.c >> +++ b/gcc/stor-layout.c >> @@ -383,6 +383,7 @@ int_mode_for_mode (enum machine_mode mode) >> case MODE_VECTOR_ACCUM: >> case MODE_VECTOR_UFRACT: >> case MODE_VECTOR_UACCUM: >> + case MODE_BOUND: >> mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0); >> break; >> >> @@ -2135,6 +2136,13 @@ layout_type (tree type) >> SET_TYPE_MODE (type, VOIDmode); >> break; >> >> + case BOUND_TYPE: >> + SET_TYPE_MODE (type, >> + mode_for_size (TYPE_PRECISION (type), MODE_BOUND, 0)); >> + TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type))); >> + TYPE_SIZE_UNIT (type) = size_int (GET_MODE_SIZE (TYPE_MODE (type))); >> + break; >> + >> case OFFSET_TYPE: >> TYPE_SIZE (type) = bitsize_int (POINTER_SIZE); >> TYPE_SIZE_UNIT (type) = size_int (POINTER_SIZE / BITS_PER_UNIT); >> diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c >> index 69e4006..8b0825c 100644 >> --- a/gcc/tree-pretty-print.c >> +++ b/gcc/tree-pretty-print.c >> @@ -697,6 +697,7 @@ dump_generic_node (pretty_printer *buffer, tree node, int spc, int flags, >> break; >> >> case VOID_TYPE: >> + case BOUND_TYPE: >> case INTEGER_TYPE: >> case REAL_TYPE: >> case FIXED_POINT_TYPE: >> diff --git a/gcc/tree.c b/gcc/tree.c >> index b469b97..bbbe16e 100644 >> --- a/gcc/tree.c >> +++ b/gcc/tree.c >> @@ -1197,6 +1197,7 @@ build_int_cst_wide (tree type, unsigned HOST_WIDE_INT low, HOST_WIDE_INT hi) >> >> case INTEGER_TYPE: >> case OFFSET_TYPE: >> + case BOUND_TYPE: >> if (TYPE_UNSIGNED (type)) >> { >> /* Cache 0..N */ >> @@ -3232,6 +3233,7 @@ type_contains_placeholder_1 (const_tree type) >> switch (TREE_CODE (type)) >> { >> case VOID_TYPE: >> + case BOUND_TYPE: >> case COMPLEX_TYPE: >> case ENUMERAL_TYPE: >> case BOOLEAN_TYPE: >> diff --git a/gcc/tree.def b/gcc/tree.def >> index f825aad..b01cdd5 100644 >> --- a/gcc/tree.def >> +++ b/gcc/tree.def >> @@ -232,6 +232,11 @@ DEFTREECODE (QUAL_UNION_TYPE, "qual_union_type", tcc_type, 0) >> /* The void type in C */ >> DEFTREECODE (VOID_TYPE, "void_type", tcc_type, 0) >> >> +/* Type to hold bounds for a pointer. >> + Has TYPE_PRECISION component to specify number of bits used >> + by this type. */ >> +DEFTREECODE (BOUND_TYPE, "bound_type", tcc_type, 0) >> + >> /* Type of functions. Special fields: >> TREE_TYPE type of value returned. >> TYPE_ARG_TYPES list of types of arguments expected. >> diff --git a/gcc/tree.h b/gcc/tree.h >> index 83edaba..863a204 100644 >> --- a/gcc/tree.h >> +++ b/gcc/tree.h >> @@ -1097,6 +1097,10 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, >> /* Nonzero if this type is a complete type. */ >> #define COMPLETE_TYPE_P(NODE) (TYPE_SIZE (NODE) != NULL_TREE) >> >> +/* Nonzero if this type is a bound type. */ >> +#define BOUND_TYPE_P(NODE) \ >> + (TREE_CODE (NODE) == BOUND_TYPE) >> + >> /* Nonzero if this type is the (possibly qualified) void type. */ >> #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE) >> >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index 0504eeb..2b5305b 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -4703,6 +4703,7 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align) >> case REFERENCE_TYPE: >> case OFFSET_TYPE: >> case FIXED_POINT_TYPE: >> + case BOUND_TYPE: >> if (! assemble_integer (expand_expr (exp, NULL_RTX, VOIDmode, >> EXPAND_INITIALIZER), >> MIN (size, thissize), align, 0))
On 10/15/13 07:31, Ilya Enkovich wrote: > Hey guys, > > could please someone look at this small patch? It blocks approved MPX > ISA support on i386 target. >>> >>> diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi >>> index 1d62223..02b1214 100644 >>> --- a/gcc/doc/rtl.texi >>> +++ b/gcc/doc/rtl.texi >>> @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the @file{@var{machine}-modes.def}. >>> @xref{Jump Patterns}, >>> also see @ref{Condition Code}. >>> >>> +@findex MODE_BOUND >>> +@item MODE_BOUND >>> +Bound modes class. Used to represent values of pointer bounds. >>> + >>> @findex MODE_RANDOM >>> @item MODE_RANDOM >>> This is a catchall mode class for modes which don't fit into the above So why are bounds distinct modes? Is there some inherent reason why bounds are something other than an integer mode (MODE_INT)? Similarly what's the rationale behind having new types for bounds? Is there some reason why they couldn't be implemented with one of the existing types? ISTM the entire patch is gated on being able to answer those two questions. jeff
2013/10/21 Jeff Law <law@redhat.com>: > On 10/15/13 07:31, Ilya Enkovich wrote: >> >> Hey guys, >> >> could please someone look at this small patch? It blocks approved MPX >> ISA support on i386 target. > > >>>> >>>> diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi >>>> index 1d62223..02b1214 100644 >>>> --- a/gcc/doc/rtl.texi >>>> +++ b/gcc/doc/rtl.texi >>>> @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the >>>> @file{@var{machine}-modes.def}. >>>> @xref{Jump Patterns}, >>>> also see @ref{Condition Code}. >>>> >>>> +@findex MODE_BOUND >>>> +@item MODE_BOUND >>>> +Bound modes class. Used to represent values of pointer bounds. >>>> + >>>> @findex MODE_RANDOM >>>> @item MODE_RANDOM >>>> This is a catchall mode class for modes which don't fit into the above > > So why are bounds distinct modes? Is there some inherent reason why > bounds are something other than an integer mode (MODE_INT)? > > Similarly what's the rationale behind having new types for bounds? Is there > some reason why they couldn't be implemented with one of the existing types? > > ISTM the entire patch is gated on being able to answer those two questions. > Hello Jeff, Before introducing new type and mode we tried to implement everything using existing ones. We tried integers, pointers, complex with pointer type as base and also structure of two pointers. The problem is that semantics of bounds is different from everything we have for base types. All operators (exprs) we have for existing types are not applicable to bounds. We probably may use some existing type/mode but it would still require some additional flag to mark bounds. And almost each first time we handle chosen basic type, it would be required to check if we are working with bounds. I do not think many GCC developers (at least in the nearest future) will care about instrumented code while writing their patches. It means that many developers may break instrumented code by adding any sort of manipulation with values of type/mode we choose as basic for bounds. I'm sure having a proper type is much more convenient and natural. In addition to all said for bound type, bound mode may also have different binary format. On i386 target bounds have special binary format, it is not equal to pair of pointers. In many places (ABI, insn templates etc.) we need to know if we work with bounds. E. g. passing 'long long' and bounds on a register(s) is different even if size is the same. Shortly: why to use same base type/mode for totally different matters? I do not know if it is possible to implement everything using existing types and modes. Probably it is possible, but for me it does not seem a right way to go. Thanks, Ilya > > jeff >
On 10/21/2013 11:10 AM, Jeff Law wrote: > So why are bounds distinct modes? Is there some inherent reason why bounds > are something other than an integer mode (MODE_INT)? I suggested the distinct modes during the NDA phase. The primary reason for this is that MPX is designed to be kind of backward compatible with previous ISAs, operating as nops. Thus we cannot allow the compiler to use the MPX registers for anything besides implementing the bounds checking. The only way I could think to positively ensure that normal operations didn't get implemented via mpx insns is to describe the new patterns with distinct modes. r~
On 10/22/13 13:12, Richard Henderson wrote: > On 10/21/2013 11:10 AM, Jeff Law wrote: >> So why are bounds distinct modes? Is there some inherent reason why bounds >> are something other than an integer mode (MODE_INT)? > > I suggested the distinct modes during the NDA phase. > > The primary reason for this is that MPX is designed to be kind of > backward compatible with previous ISAs, operating as nops. Thus > we cannot allow the compiler to use the MPX registers for anything > besides implementing the bounds checking. Right. > > The only way I could think to positively ensure that normal operations > didn't get implemented via mpx insns is to describe the new patterns > with distinct modes. Presumably once we have a distinct mode, we do the right magic in HARD_REGNO_MODE_OK and that's how you get your guarantee. I'm assuming we're exposing these to the register allocator (I haven't looked at the full series yet). Presumably you need a distinct mode coming out of the front-end/gimple to ensure we get the new mode in RTL. It all seems reasonable -- I wasn't asking Ilya to change anything, I'm just trying to understand the rationale before going any further with the patch and this helps considerably. jeff
On 10/22/2013 12:18 PM, Jeff Law wrote: >> The only way I could think to positively ensure that normal operations >> didn't get implemented via mpx insns is to describe the new patterns >> with distinct modes. > Presumably once we have a distinct mode, we do the right magic in > HARD_REGNO_MODE_OK and that's how you get your guarantee. I'm assuming we're > exposing these to the register allocator (I haven't looked at the full series > yet). Yeah, the register allocator was supposed to be involved. (And I need to review the series myself.) > Presumably you need a distinct mode coming out of the front-end/gimple to > ensure we get the new mode in RTL. Yes, which is where I believe the new types come from as well. r~
On 10/22/13 13:31, Richard Henderson wrote: > > Yes, which is where I believe the new types come from as well. OK. Thanks for clarifying. I'm about to go offline for a few hours, but will start working my way through the MPX stuff. jeff
On 09/17/13 02:18, Ilya Enkovich wrote: > Hi, > > Here is a patch introducing new type and mode for bounds. It is a part of MPX ISA support patch (http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01094.html). > > Bootstrapped and tested on linux-x86_64. Is it OK for trunk? > > Thanks, > Ilya > -- > > gcc/ > > 2013-09-16 Ilya Enkovich <ilya.enkovich@intel.com> > > * mode-classes.def (MODE_BOUND): New. > * tree.def (BOUND_TYPE): New. > * genmodes.c (complete_mode): Support MODE_BOUND. > (BOUND_MODE): New. > (make_bound_mode): New. > * machmode.h (BOUND_MODE_P): New. > * stor-layout.c (int_mode_for_mode): Support MODE_BOUND. > (layout_type): Support BOUND_TYPE. > * tree-pretty-print.c (dump_generic_node): Support BOUND_TYPE. > * tree.c (build_int_cst_wide): Support BOUND_TYPE. > (type_contains_placeholder_1): Likewise. > * tree.h (BOUND_TYPE_P): New. > * varasm.c (output_constant): Support BOUND_TYPE. > * doc/rtl.texi (MODE_BOUND): New. Mostly OK. Just a few minor things that should be fixed or at least clarified. > > diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi > index 1d62223..02b1214 100644 > --- a/gcc/doc/rtl.texi > +++ b/gcc/doc/rtl.texi > @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the @file{@var{machine}-modes.def}. > @xref{Jump Patterns}, > also see @ref{Condition Code}. > > +@findex MODE_BOUND > +@item MODE_BOUND > +Bound modes class. Used to represent values of pointer bounds. I can't help but feel more is needed here -- without going into the details of the MPX implementation we ought to say something about how these differ from the more normal integer modes. Drawing from the brief discussion between Richard & myself earlier today should give some ideas on how to improve this. I'd probably use MODE_POINTER_BOUNDS which is a bit more descriptive. We wouldn't want someone to (for example) think this stuff relates to array bounds. Obviously a change to MODE_POINTER_BOUNDS would propagate into other places where you use "BOUND" without a "POINTER" qualification, such as "BOUND_MODE_P" which we'd change to POINTER_BOUNDS_MODE_P. > diff --git a/gcc/mode-classes.def b/gcc/mode-classes.def > index 7207ef7..c5ea215 100644 > --- a/gcc/mode-classes.def > +++ b/gcc/mode-classes.def > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see > DEF_MODE_CLASS (MODE_RANDOM), /* other */ \ > DEF_MODE_CLASS (MODE_CC), /* condition code in a register */ \ > DEF_MODE_CLASS (MODE_INT), /* integer */ \ > + DEF_MODE_CLASS (MODE_BOUND), /* bounds */ \ > DEF_MODE_CLASS (MODE_PARTIAL_INT), /* integer with padding bits */ \ > DEF_MODE_CLASS (MODE_FRACT), /* signed fractional number */ \ > DEF_MODE_CLASS (MODE_UFRACT), /* unsigned fractional number */ \ Does genmodes do the right thing WRT MAX_INT_MODE and MIN_INT_MODE? I'd be more comfortable if MODE_POINTER_BOUNDS wasn't sitting between MODE_INT and MODE_PARTIAL_INT. I'm not aware of code that iterates over these things that would get confused, but ISTM putting MODE_POINTER_BOUNDS after MODE_PARTIAL_INT is marginally safer. > diff --git a/gcc/tree.c b/gcc/tree.c > index b469b97..bbbe16e 100644 > --- a/gcc/tree.c > +++ b/gcc/tree.c > @@ -1197,6 +1197,7 @@ build_int_cst_wide (tree type, unsigned HOST_WIDE_INT low, HOST_WIDE_INT hi) > > case INTEGER_TYPE: > case OFFSET_TYPE: > + case BOUND_TYPE: > if (TYPE_UNSIGNED (type)) > { > /* Cache 0..N */ So here you're effectively treading POINTER_BOUNDS_TYPE like an integer. I'm guessing there's a number of flags that may not be relevant for your type and which you might want to repurpose (again, I haven't looked at the entire patchset). If so, you want to be real careful here since you'll be looking at (for example) TYPE_UNSIGNED which may not have any real meaning for POINTER_BOUNDS_TYPE. Overall, it seems fairly reasonable -- the biggest concern of mine is in the last comment. Are you going to be repurposing various flag bits in the type? If so, then we have to be more careful in code like above. Jeff
diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi index 1d62223..02b1214 100644 --- a/gcc/doc/rtl.texi +++ b/gcc/doc/rtl.texi @@ -1382,6 +1382,10 @@ any @code{CC_MODE} modes listed in the @file{@var{machine}-modes.def}. @xref{Jump Patterns}, also see @ref{Condition Code}. +@findex MODE_BOUND +@item MODE_BOUND +Bound modes class. Used to represent values of pointer bounds. + @findex MODE_RANDOM @item MODE_RANDOM This is a catchall mode class for modes which don't fit into the above diff --git a/gcc/genmodes.c b/gcc/genmodes.c index dc38483..89174ec 100644 --- a/gcc/genmodes.c +++ b/gcc/genmodes.c @@ -333,6 +333,7 @@ complete_mode (struct mode_data *m) break; case MODE_INT: + case MODE_BOUND: case MODE_FLOAT: case MODE_DECIMAL_FLOAT: case MODE_FRACT: @@ -533,6 +534,18 @@ make_special_mode (enum mode_class cl, const char *name, new_mode (cl, name, file, line); } +#define BOUND_MODE(N, Y) make_bound_mode (#N, Y, __FILE__, __LINE__) + +static void ATTRIBUTE_UNUSED +make_bound_mode (const char *name, + unsigned int bytesize, + const char *file, unsigned int line) +{ + struct mode_data *m = new_mode (MODE_BOUND, name, file, line); + m->bytesize = bytesize; +} + + #define INT_MODE(N, Y) FRACTIONAL_INT_MODE (N, -1U, Y) #define FRACTIONAL_INT_MODE(N, B, Y) \ make_int_mode (#N, B, Y, __FILE__, __LINE__) diff --git a/gcc/machmode.h b/gcc/machmode.h index 981ee92..d4a20b2 100644 --- a/gcc/machmode.h +++ b/gcc/machmode.h @@ -174,6 +174,9 @@ extern const unsigned char mode_class[NUM_MACHINE_MODES]; || CLASS == MODE_ACCUM \ || CLASS == MODE_UACCUM) +#define BOUND_MODE_P(MODE) \ + (GET_MODE_CLASS (MODE) == MODE_BOUND) + /* Get the size in bytes and bits of an object of mode MODE. */ extern CONST_MODE_SIZE unsigned char mode_size[NUM_MACHINE_MODES]; diff --git a/gcc/mode-classes.def b/gcc/mode-classes.def index 7207ef7..c5ea215 100644 --- a/gcc/mode-classes.def +++ b/gcc/mode-classes.def @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see DEF_MODE_CLASS (MODE_RANDOM), /* other */ \ DEF_MODE_CLASS (MODE_CC), /* condition code in a register */ \ DEF_MODE_CLASS (MODE_INT), /* integer */ \ + DEF_MODE_CLASS (MODE_BOUND), /* bounds */ \ DEF_MODE_CLASS (MODE_PARTIAL_INT), /* integer with padding bits */ \ DEF_MODE_CLASS (MODE_FRACT), /* signed fractional number */ \ DEF_MODE_CLASS (MODE_UFRACT), /* unsigned fractional number */ \ diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c index 6f6b310..82611c7 100644 --- a/gcc/stor-layout.c +++ b/gcc/stor-layout.c @@ -383,6 +383,7 @@ int_mode_for_mode (enum machine_mode mode) case MODE_VECTOR_ACCUM: case MODE_VECTOR_UFRACT: case MODE_VECTOR_UACCUM: + case MODE_BOUND: mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0); break; @@ -2135,6 +2136,13 @@ layout_type (tree type) SET_TYPE_MODE (type, VOIDmode); break; + case BOUND_TYPE: + SET_TYPE_MODE (type, + mode_for_size (TYPE_PRECISION (type), MODE_BOUND, 0)); + TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type))); + TYPE_SIZE_UNIT (type) = size_int (GET_MODE_SIZE (TYPE_MODE (type))); + break; + case OFFSET_TYPE: TYPE_SIZE (type) = bitsize_int (POINTER_SIZE); TYPE_SIZE_UNIT (type) = size_int (POINTER_SIZE / BITS_PER_UNIT); diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index 69e4006..8b0825c 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -697,6 +697,7 @@ dump_generic_node (pretty_printer *buffer, tree node, int spc, int flags, break; case VOID_TYPE: + case BOUND_TYPE: case INTEGER_TYPE: case REAL_TYPE: case FIXED_POINT_TYPE: diff --git a/gcc/tree.c b/gcc/tree.c index b469b97..bbbe16e 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -1197,6 +1197,7 @@ build_int_cst_wide (tree type, unsigned HOST_WIDE_INT low, HOST_WIDE_INT hi) case INTEGER_TYPE: case OFFSET_TYPE: + case BOUND_TYPE: if (TYPE_UNSIGNED (type)) { /* Cache 0..N */ @@ -3232,6 +3233,7 @@ type_contains_placeholder_1 (const_tree type) switch (TREE_CODE (type)) { case VOID_TYPE: + case BOUND_TYPE: case COMPLEX_TYPE: case ENUMERAL_TYPE: case BOOLEAN_TYPE: diff --git a/gcc/tree.def b/gcc/tree.def index f825aad..b01cdd5 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -232,6 +232,11 @@ DEFTREECODE (QUAL_UNION_TYPE, "qual_union_type", tcc_type, 0) /* The void type in C */ DEFTREECODE (VOID_TYPE, "void_type", tcc_type, 0) +/* Type to hold bounds for a pointer. + Has TYPE_PRECISION component to specify number of bits used + by this type. */ +DEFTREECODE (BOUND_TYPE, "bound_type", tcc_type, 0) + /* Type of functions. Special fields: TREE_TYPE type of value returned. TYPE_ARG_TYPES list of types of arguments expected. diff --git a/gcc/tree.h b/gcc/tree.h index 83edaba..863a204 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -1097,6 +1097,10 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, /* Nonzero if this type is a complete type. */ #define COMPLETE_TYPE_P(NODE) (TYPE_SIZE (NODE) != NULL_TREE) +/* Nonzero if this type is a bound type. */ +#define BOUND_TYPE_P(NODE) \ + (TREE_CODE (NODE) == BOUND_TYPE) + /* Nonzero if this type is the (possibly qualified) void type. */ #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE) diff --git a/gcc/varasm.c b/gcc/varasm.c index 0504eeb..2b5305b 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -4703,6 +4703,7 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align) case REFERENCE_TYPE: case OFFSET_TYPE: case FIXED_POINT_TYPE: + case BOUND_TYPE: if (! assemble_integer (expand_expr (exp, NULL_RTX, VOIDmode, EXPAND_INITIALIZER), MIN (size, thissize), align, 0))