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
