This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]