diff mbox

[i386,MPX,1/X] Support of Intel MPX ISA. 1/2 Bound type and modes

Message ID 20131023105758.GA30896@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Oct. 23, 2013, 10:57 a.m. UTC 
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.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

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.enkovich@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.

Comments

Jeff Law Oct. 23, 2013, 8:03 p.m. UTC | #1 
On 10/23/13 04:57, Ilya Enkovich wrote:
>
> 2013-10-23  Ilya Enkovich  <ilya.enkovich@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.
OK for the trunk.  IIRC, there was a backend patch with conditional 
approval that should be good to go now (conditional upon the acceptance 
of the types/modes patch).

Note that since I asked for a couple things to be renamed that backend 
patch might need tweaking.  If so, make the obvious changes, post the 
patch (so it's recorded into the archives) and go ahead and check it 
into the trunk.

jeff
Ilya Enkovich Oct. 24, 2013, 8:46 a.m. UTC | #2 
2013/10/24 Jeff Law <law@redhat.com>:
> On 10/23/13 04:57, Ilya Enkovich wrote:
>>
>>
>> 2013-10-23  Ilya Enkovich  <ilya.enkovich@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.
>
> OK for the trunk.  IIRC, there was a backend patch with conditional approval
> that should be good to go now (conditional upon the acceptance of the
> types/modes patch).
>
> Note that since I asked for a couple things to be renamed that backend patch
> might need tweaking.  If so, make the obvious changes, post the patch (so
> it's recorded into the archives) and go ahead and check it into the trunk.
>
> jeff
>

Thanks!

Ilya
diff mbox

Patch

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))