eOn 22 Oct 22:55, Jeff Law wrote:
> 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.enkov...@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
Thanks for review!
You are right here, treating bounds as integer is wrong. Currently we do not
use type flags for bounds, checking TYPE_UNSIGNED is incorrect. Bounds constant
in this case is better to be treated as pointer constant when only zero value
is a special case.
Below is the a new version with all required changes.
Ilya
--
gcc/
2013-10-23 Ilya Enkovich <ilya.enkov...@intel.com>
* mode-classes.def (MODE_POINTER_BOUNDS): New.
* tree.def (POINTER_BOUNDS_TYPE): New.
* genmodes.c (complete_mode): Support MODE_POINTER_BOUNDS.
(POINTER_BOUNDS_MODE): New.
(make_pointer_bounds_mode): New.
* machmode.h (POINTER_BOUNDS_MODE_P): New.
* stor-layout.c (int_mode_for_mode): Support MODE_POINTER_BOUNDS.
(layout_type): Support POINTER_BOUNDS_TYPE.
* tree-pretty-print.c (dump_generic_node): Support POINTER_BOUNDS_TYPE.
* tree.c (build_int_cst_wide): Support POINTER_BOUNDS_TYPE.
(type_contains_placeholder_1): Likewise.
* tree.h (POINTER_BOUNDS_TYPE_P): New.
* varasm.c (output_constant): Support POINTER_BOUNDS_TYPE.
* doc/rtl.texi (MODE_POINTER_BOUNDS): New.
diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index 84c0444..77a9c70 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -1382,6 +1382,12 @@ any @code{CC_MODE} modes listed in the
@file{@var{machine}-modes.def}.
@xref{Jump Patterns},
also see @ref{Condition Code}.
+@findex MODE_POINTER_BOUNDS
+@item MODE_POINTER_BOUNDS
+Pointer bounds modes. Used to represent values of pointer bounds type.
+Operations in these modes may be executed as NOPs depending on hardware
+features and environment setup.
+
@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 a0b2f21..0aa5de6 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_POINTER_BOUNDS:
case MODE_FLOAT:
case MODE_DECIMAL_FLOAT:
case MODE_FRACT:
@@ -534,6 +535,19 @@ make_special_mode (enum mode_class cl, const char *name,
new_mode (cl, name, file, line);
}
+#define POINTER_BOUNDS_MODE(N, Y) \
+ make_pointer_bounds_mode (#N, Y, __FILE__, __LINE__)
+
+static void ATTRIBUTE_UNUSED
+make_pointer_bounds_mode (const char *name,
+ unsigned int bytesize,
+ const char *file, unsigned int line)
+{
+ struct mode_data *m = new_mode (MODE_POINTER_BOUNDS, 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..ede44d5 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 POINTER_BOUNDS_MODE_P(MODE) \
+ (GET_MODE_CLASS (MODE) == MODE_POINTER_BOUNDS)
+
/* 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..a94fd61 100644
--- a/gcc/mode-classes.def
+++ b/gcc/mode-classes.def
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see
DEF_MODE_CLASS (MODE_CC), /* condition code in a register */ \
DEF_MODE_CLASS (MODE_INT), /* integer */ \
DEF_MODE_CLASS (MODE_PARTIAL_INT), /* integer with padding bits */ \
+ DEF_MODE_CLASS (MODE_POINTER_BOUNDS), /* bounds */ \
DEF_MODE_CLASS (MODE_FRACT), /* signed fractional number */ \
DEF_MODE_CLASS (MODE_UFRACT), /* unsigned fractional number
*/ \
DEF_MODE_CLASS (MODE_ACCUM), /* signed accumulator */ \
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 0299836..a28a448 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_POINTER_BOUNDS:
mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0);
break;
@@ -2135,6 +2136,14 @@ layout_type (tree type)
SET_TYPE_MODE (type, VOIDmode);
break;
+ case POINTER_BOUNDS_TYPE:
+ SET_TYPE_MODE (type,
+ mode_for_size (TYPE_PRECISION (type),
+ MODE_POINTER_BOUNDS, 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 c357b06..486046c 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 POINTER_BOUNDS_TYPE:
case INTEGER_TYPE:
case REAL_TYPE:
case FIXED_POINT_TYPE:
diff --git a/gcc/tree.c b/gcc/tree.c
index ebee116..85380a1 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -1180,7 +1180,8 @@ build_int_cst_wide (tree type, unsigned HOST_WIDE_INT
low, HOST_WIDE_INT hi)
case POINTER_TYPE:
case REFERENCE_TYPE:
- /* Cache NULL pointer. */
+ case POINTER_BOUNDS_TYPE:
+ /* Cache NULL pointer and zero bounds. */
if (!hi && !low)
{
limit = 1;
@@ -3232,6 +3233,7 @@ type_contains_placeholder_1 (const_tree type)
switch (TREE_CODE (type))
{
case VOID_TYPE:
+ case POINTER_BOUNDS_TYPE:
case COMPLEX_TYPE:
case ENUMERAL_TYPE:
case BOOLEAN_TYPE:
diff --git a/gcc/tree.def b/gcc/tree.def
index f825aad..fc2770e 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 (POINTER_BOUNDS_TYPE, "pointer_bounds_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 4cd7e9e..b864ae3 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -542,6 +542,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 pointer bounds type. */
+#define POINTER_BOUNDS_TYPE_P(NODE) \
+ (TREE_CODE (NODE) == POINTER_BOUNDS_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 12fb7c4..233a9e1 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 POINTER_BOUNDS_TYPE:
if (! assemble_integer (expand_expr (exp, NULL_RTX, VOIDmode,
EXPAND_INITIALIZER),
MIN (size, thissize), align, 0))
