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


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


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