Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
2013/10/24 Jeff Law l...@redhat.com: On 10/23/13 04:57, Ilya Enkovich wrote: 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. 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
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On 01 Oct 20:00, Uros Bizjak wrote:
This is OK for mainline, on the condition that target independent part
is approved and committed first.
Thanks,
Uros.
Thanks for review!
Attached is a version to be committed. The only difference from the previous
one is BOUND_MODE renamed to POINTER_BOUNDS_MODE due to changes in target
independent part. ChangeLog was not modified.
Ilya
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 92e0c05..ddfd402 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -18,7 +18,7 @@
;; http://www.gnu.org/licenses/.
;;; Unused letters:
-;;; B H T
+;;; H
;;; h j
;; Integer register constraints.
@@ -91,6 +91,9 @@
(define_register_constraint x TARGET_SSE ? SSE_REGS : NO_REGS
Any SSE register.)
+(define_register_constraint B TARGET_MPX ? BND_REGS : NO_REGS
+ @internal Any bound register.)
+
;; We use the Y prefix to denote any number of conditional register sets:
;; z First SSE register.
;; i SSE2 inter-unit moves to SSE register enabled
@@ -232,3 +235,15 @@
to fit that range (for immediate operands in zero-extending x86-64
instructions).
(match_operand 0 x86_64_zext_immediate_operand))
+
+;; T prefix is used for different address constraints
+;; i - address with no index and no rip
+;; b - address with no base and no rip
+
+(define_address_constraint Ti
+ MPX address operand without index
+ (match_operand 0 address_mpx_no_index_operand))
+
+(define_address_constraint Tb
+ MPX address operand without base
+ (match_operand 0 address_mpx_no_base_operand))
diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 2e764e7..da16240 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -358,6 +358,8 @@ ix86_target_macros_internal (HOST_WIDE_INT isa_flag,
def_or_undef (parse_in, __SSE_MATH__);
if ((fpmath FPMATH_SSE) (isa_flag OPTION_MASK_ISA_SSE2))
def_or_undef (parse_in, __SSE2_MATH__);
+ if (isa_flag OPTION_MASK_ISA_MPX)
+def_or_undef (parse_in, __MPX__);
}
diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def
index e0b8fc8..a73730e 100644
--- a/gcc/config/i386/i386-modes.def
+++ b/gcc/config/i386/i386-modes.def
@@ -87,6 +87,9 @@ VECTOR_MODE (INT, DI, 1); /* V1DI */
VECTOR_MODE (INT, SI, 1); /* V1SI */
VECTOR_MODE (INT, QI, 2); /* V2QI */
+POINTER_BOUNDS_MODE (BND32, 8);
+POINTER_BOUNDS_MODE (BND64, 16);
+
INT_MODE (OI, 32);
INT_MODE (XI, 64);
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 3ab2f3a..fdd98fc 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -239,6 +239,8 @@ extern void ix86_expand_mul_widen_hilo (rtx, rtx, rtx,
bool, bool);
extern void ix86_expand_sse2_mulv4si3 (rtx, rtx, rtx);
extern void ix86_expand_sse2_mulvxdi3 (rtx, rtx, rtx);
+extern bool ix86_bnd_prefixed_insn_p (rtx);
+
/* In i386-c.c */
extern void ix86_target_macros (void);
extern void ix86_register_pragmas (void);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 21fc531..f973f84 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1956,6 +1956,8 @@ enum reg_class const regclass_map[FIRST_PSEUDO_REGISTER] =
/* Mask registers. */
MASK_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
+ /* MPX bound registers */
+ BND_REGS, BND_REGS, BND_REGS, BND_REGS,
};
/* The default register map used in 32bit mode. */
@@ -1972,6 +1974,7 @@ int const dbx_register_map[FIRST_PSEUDO_REGISTER] =
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 16-23*/
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 24-31*/
93, 94, 95, 96, 97, 98, 99, 100, /* Mask registers */
+ 101, 102, 103, 104, /* bound registers */
};
/* The default register map used in 64bit mode. */
@@ -1988,6 +1991,7 @@ int const dbx64_register_map[FIRST_PSEUDO_REGISTER] =
67, 68, 69, 70, 71, 72, 73, 74, /* AVX-512 registers 16-23 */
75, 76, 77, 78, 79, 80, 81, 82, /* AVX-512 registers 24-31 */
118, 119, 120, 121, 122, 123, 124, 125, /* Mask registers */
+ 126, 127, 128, 129, /* bound registers */
};
/* Define the register numbers to be used in Dwarf debugging information.
@@ -2056,6 +2060,7 @@ int const svr4_dbx_register_map[FIRST_PSEUDO_REGISTER] =
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 16-23*/
-1, -1, -1, -1, -1, -1, -1, -1, /* AVX-512 registers 24-31*/
93, 94, 95, 96, 97, 98, 99, 100, /* Mask registers */
+ -1, -1, -1, -1, /* bound registers */
};
/* Define parameter passing and return registers. */
@@ -2478,6 +2483,7 @@ ix86_target_string (HOST_WIDE_INT isa, int flags, const
char *arch,
{ -mrtm,
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On Thu, Oct 24, 2013 at 12:06 PM, Ilya Enkovich enkovich@gmail.com wrote:
On 01 Oct 20:00, Uros Bizjak wrote:
This is OK for mainline, on the condition that target independent part
is approved and committed first.
Thanks,
Uros.
Thanks for review!
Attached is a version to be committed. The only difference from the previous
one is BOUND_MODE renamed to POINTER_BOUNDS_MODE due to changes in target
independent part. ChangeLog was not modified.
I think you missed a couple of length - length_nobnd updates:
@@ -11635,7 +11680,12 @@
[(simple_return)
(unspec [(const_int 0)] UNSPEC_REP)]
reload_completed
- rep%; ret
+{
+ if (ix86_bnd_prefixed_insn_p (insn))
+return %!ret;
+
+ return rep%; ret;
+}
[(set_attr length 2)
(set_attr atom_unit jeu)
(set_attr length_immediate 0)
and possibly here:
@@ -11186,7 +11231,7 @@
(define_insn *indirect_jump
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
@@ -11235,7 +11280,7 @@
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))
(use (label_ref (match_operand 1)))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
Uros.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
2013/10/24 Uros Bizjak ubiz...@gmail.com:
On Thu, Oct 24, 2013 at 12:06 PM, Ilya Enkovich enkovich@gmail.com
wrote:
On 01 Oct 20:00, Uros Bizjak wrote:
This is OK for mainline, on the condition that target independent part
is approved and committed first.
Thanks,
Uros.
Thanks for review!
Attached is a version to be committed. The only difference from the
previous one is BOUND_MODE renamed to POINTER_BOUNDS_MODE due to changes in
target independent part. ChangeLog was not modified.
I think you missed a couple of length - length_nobnd updates:
@@ -11635,7 +11680,12 @@
[(simple_return)
(unspec [(const_int 0)] UNSPEC_REP)]
reload_completed
- rep%; ret
+{
+ if (ix86_bnd_prefixed_insn_p (insn))
+return %!ret;
+
+ return rep%; ret;
+}
[(set_attr length 2)
(set_attr atom_unit jeu)
(set_attr length_immediate 0)
There is no reason for length_nobnd because instruction length is
always 2. Difference is in prefix used for MPX and non-MPX code.
and possibly here:
@@ -11186,7 +11231,7 @@
(define_insn *indirect_jump
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
@@ -11235,7 +11280,7 @@
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))
(use (label_ref (match_operand 1)))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
For these cases 'prefix_rep' attribute makes the work due to 'ibr'
type. Generic length should work fine.
Thanks,
Ilya
Uros.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On Thu, Oct 24, 2013 at 1:29 PM, Ilya Enkovich enkovich@gmail.com wrote:
This is OK for mainline, on the condition that target independent part
is approved and committed first.
Thanks,
Uros.
Thanks for review!
Attached is a version to be committed. The only difference from the
previous one is BOUND_MODE renamed to POINTER_BOUNDS_MODE due to changes in
target independent part. ChangeLog was not modified.
I think you missed a couple of length - length_nobnd updates:
@@ -11635,7 +11680,12 @@
[(simple_return)
(unspec [(const_int 0)] UNSPEC_REP)]
reload_completed
- rep%; ret
+{
+ if (ix86_bnd_prefixed_insn_p (insn))
+return %!ret;
+
+ return rep%; ret;
+}
[(set_attr length 2)
(set_attr atom_unit jeu)
(set_attr length_immediate 0)
There is no reason for length_nobnd because instruction length is
always 2. Difference is in prefix used for MPX and non-MPX code.
and possibly here:
@@ -11186,7 +11231,7 @@
(define_insn *indirect_jump
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
@@ -11235,7 +11280,7 @@
[(set (pc) (match_operand:W 0 indirect_branch_operand rw))
(use (label_ref (match_operand 1)))]
- jmp\t%A0
+ %!jmp\t%A0
[(set_attr type ibr)
(set_attr length_immediate 0)])
For these cases 'prefix_rep' attribute makes the work due to 'ibr'
type. Generic length should work fine.
Indeed. Thanks for explanation!
Uros.
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.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
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/23/13 04:57, Ilya Enkovich wrote: 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. 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
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
2013/10/21 Jeff Law l...@redhat.com:
On 10/15/13 07:31, Ilya Enkovich wrote:
Hey guys,
could please someone look at this small patch? It blocks approved MPX
ISA support on i386 target.
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.
+
@findex MODE_RANDOM
@item MODE_RANDOM
This is a catchall mode class for modes which don't fit into the above
So why are bounds distinct modes?Is there some inherent reason why
bounds are something other than an integer mode (MODE_INT)?
Similarly what's the rationale behind having new types for bounds? Is there
some reason why they couldn't be implemented with one of the existing types?
ISTM the entire patch is gated on being able to answer those two questions.
Hello Jeff,
Before introducing new type and mode we tried to implement everything
using existing ones. We tried integers, pointers, complex with pointer
type as base and also structure of two pointers. The problem is that
semantics of bounds is different from everything we have for base
types. All operators (exprs) we have for existing types are not
applicable to bounds. We probably may use some existing type/mode but
it would still require some additional flag to mark bounds. And almost
each first time we handle chosen basic type, it would be required to
check if we are working with bounds. I do not think many GCC
developers (at least in the nearest future) will care about
instrumented code while writing their patches. It means that many
developers may break instrumented code by adding any sort of
manipulation with values of type/mode we choose as basic for bounds.
I'm sure having a proper type is much more convenient and natural.
In addition to all said for bound type, bound mode may also have
different binary format. On i386 target bounds have special binary
format, it is not equal to pair of pointers. In many places (ABI, insn
templates etc.) we need to know if we work with bounds. E. g. passing
'long long' and bounds on a register(s) is different even if size is
the same.
Shortly: why to use same base type/mode for totally different matters?
I do not know if it is possible to implement everything using existing
types and modes. Probably it is possible, but for me it does not seem
a right way to go.
Thanks,
Ilya
jeff
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/21/2013 11:10 AM, Jeff Law wrote: So why are bounds distinct modes?Is there some inherent reason why bounds are something other than an integer mode (MODE_INT)? I suggested the distinct modes during the NDA phase. The primary reason for this is that MPX is designed to be kind of backward compatible with previous ISAs, operating as nops. Thus we cannot allow the compiler to use the MPX registers for anything besides implementing the bounds checking. The only way I could think to positively ensure that normal operations didn't get implemented via mpx insns is to describe the new patterns with distinct modes. r~
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/22/13 13:12, Richard Henderson wrote: On 10/21/2013 11:10 AM, Jeff Law wrote: So why are bounds distinct modes?Is there some inherent reason why bounds are something other than an integer mode (MODE_INT)? I suggested the distinct modes during the NDA phase. The primary reason for this is that MPX is designed to be kind of backward compatible with previous ISAs, operating as nops. Thus we cannot allow the compiler to use the MPX registers for anything besides implementing the bounds checking. Right. The only way I could think to positively ensure that normal operations didn't get implemented via mpx insns is to describe the new patterns with distinct modes. Presumably once we have a distinct mode, we do the right magic in HARD_REGNO_MODE_OK and that's how you get your guarantee. I'm assuming we're exposing these to the register allocator (I haven't looked at the full series yet). Presumably you need a distinct mode coming out of the front-end/gimple to ensure we get the new mode in RTL. It all seems reasonable -- I wasn't asking Ilya to change anything, I'm just trying to understand the rationale before going any further with the patch and this helps considerably. jeff
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/22/2013 12:18 PM, Jeff Law wrote: The only way I could think to positively ensure that normal operations didn't get implemented via mpx insns is to describe the new patterns with distinct modes. Presumably once we have a distinct mode, we do the right magic in HARD_REGNO_MODE_OK and that's how you get your guarantee. I'm assuming we're exposing these to the register allocator (I haven't looked at the full series yet). Yeah, the register allocator was supposed to be involved. (And I need to review the series myself.) Presumably you need a distinct mode coming out of the front-end/gimple to ensure we get the new mode in RTL. Yes, which is where I believe the new types come from as well. r~
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/22/13 13:31, Richard Henderson wrote: Yes, which is where I believe the new types come from as well. OK. Thanks for clarifying. I'm about to go offline for a few hours, but will start working my way through the MPX stuff. jeff
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.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
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
On 10/15/13 07:31, Ilya Enkovich wrote:
Hey guys,
could please someone look at this small patch? It blocks approved MPX
ISA support on i386 target.
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.
+
@findex MODE_RANDOM
@item MODE_RANDOM
This is a catchall mode class for modes which don't fit into the above
So why are bounds distinct modes?Is there some inherent reason why
bounds are something other than an integer mode (MODE_INT)?
Similarly what's the rationale behind having new types for bounds? Is
there some reason why they couldn't be implemented with one of the
existing types?
ISTM the entire patch is gated on being able to answer those two questions.
jeff
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
Hey guys,
could please someone look at this small patch? It blocks approved MPX
ISA support on i386 target.
Thanks,
Ilya
2013/10/2 Ilya Enkovich enkovich@gmail.com:
Ping
2013/9/17 Ilya Enkovich enkovich@gmail.com:
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.
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.
+
@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 dc38483..89174ec 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_BOUND:
case MODE_FLOAT:
case MODE_DECIMAL_FLOAT:
case MODE_FRACT:
@@ -533,6 +534,18 @@ make_special_mode (enum mode_class cl, const char *name,
new_mode (cl, name, file, line);
}
+#define BOUND_MODE(N, Y) make_bound_mode (#N, Y, __FILE__, __LINE__)
+
+static void ATTRIBUTE_UNUSED
+make_bound_mode (const char *name,
+ unsigned int bytesize,
+ const char *file, unsigned int line)
+{
+ struct mode_data *m = new_mode (MODE_BOUND, 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..d4a20b2 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 BOUND_MODE_P(MODE) \
+ (GET_MODE_CLASS (MODE) == MODE_BOUND)
+
/* 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..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 */ \
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 6f6b310..82611c7 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_BOUND:
mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0);
break;
@@ -2135,6 +2136,13 @@ layout_type (tree type)
SET_TYPE_MODE (type, VOIDmode);
break;
+case BOUND_TYPE:
+ SET_TYPE_MODE (type,
+ mode_for_size (TYPE_PRECISION (type), MODE_BOUND, 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 69e4006..8b0825c 100644
---
Re: [PATCH, i386, MPX, 1/X] Support of Intel MPX ISA. 1/2 Bound type and modes
Ping
2013/9/17 Ilya Enkovich enkovich@gmail.com:
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.
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.
+
@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 dc38483..89174ec 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_BOUND:
case MODE_FLOAT:
case MODE_DECIMAL_FLOAT:
case MODE_FRACT:
@@ -533,6 +534,18 @@ make_special_mode (enum mode_class cl, const char *name,
new_mode (cl, name, file, line);
}
+#define BOUND_MODE(N, Y) make_bound_mode (#N, Y, __FILE__, __LINE__)
+
+static void ATTRIBUTE_UNUSED
+make_bound_mode (const char *name,
+ unsigned int bytesize,
+ const char *file, unsigned int line)
+{
+ struct mode_data *m = new_mode (MODE_BOUND, 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..d4a20b2 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 BOUND_MODE_P(MODE) \
+ (GET_MODE_CLASS (MODE) == MODE_BOUND)
+
/* 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..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
*/ \
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 6f6b310..82611c7 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_BOUND:
mode = mode_for_size (GET_MODE_BITSIZE (mode), MODE_INT, 0);
break;
@@ -2135,6 +2136,13 @@ layout_type (tree type)
SET_TYPE_MODE (type, VOIDmode);
break;
+case BOUND_TYPE:
+ SET_TYPE_MODE (type,
+ mode_for_size (TYPE_PRECISION (type), MODE_BOUND, 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 69e4006..8b0825c 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
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On 26 Sep 23:12, Uros Bizjak wrote: On Tue, Sep 17, 2013 at 10:41 AM, Ilya Enkovich enkovich@gmail.com wrote: The x86 part looks mostly OK (I have a couple of comments bellow), but please first get target-independent changes reviewed and committed. Do you mean I should move bound type and mode declaration into a separate patch? Yes, target-independent part (middle end) has to go through the separate review to check if this part is OK. The target-dependent part uses the infrastructure from the middle end, so it can go into the code base only after target-independent parts are committed. I sent a separate patch for bound type and mode class (http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01268.html). Here is target part of the patch with fixes you mentioned. Does it look OK? Bootstrapped and checked on linux-x86_64. Still shows incorrect length attribute computation (described here http://gcc.gnu.org/ml/gcc/2013-07/msg00311.html). Please look at the attached patch that solves length computation problem. The patch also implements length calculation in a generic way, as proposed earlier. The idea is to calculate total insn length via generic length attribute calculation from length_nobnd attribute, but iff length_attribute is non-null. This way, we are able to decorate bnd-prefixed instructions by lenght_nobnd attribute, and generic part will automatically call ix86_bnd_prefixed_insn_p predicate with current insn pattern. I also belive that this approach is most flexible to decorate future patterns. The patch adds new attribute to a couple of patterns to illustrate its usage. Please test this approach. Modulo length calculations, improved by the patch in this message, I have no further comments, but please repost complete (target part) of your patch. Hi Uros, Thanks for your reply! I applied approach you proposed for length attribute. It works well. Make check is clean now. I also adjusted bound registers to recently added mask registers. Attached is a new patch. Thanks, Ilya -- 2013-09-30 Ilya Enkovich ilya.enkov...@intel.com * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-c.c (ix86_target_macros_internal): Add __MPX__. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal): Support MPX ISA. (ix86_conditional_register_usage): Support bound registers. (print_reg): Likewise. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (ix86_print_operand): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (length_nobnd): New. (length): Use length_nobnd if specified. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add bnd prefix and rename length attr to length_nobnd. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1):
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On Tue, Oct 1, 2013 at 9:41 AM, Ilya Enkovich enkovich@gmail.com wrote: The x86 part looks mostly OK (I have a couple of comments bellow), but please first get target-independent changes reviewed and committed. Do you mean I should move bound type and mode declaration into a separate patch? Yes, target-independent part (middle end) has to go through the separate review to check if this part is OK. The target-dependent part uses the infrastructure from the middle end, so it can go into the code base only after target-independent parts are committed. I sent a separate patch for bound type and mode class (http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01268.html). Here is target part of the patch with fixes you mentioned. Does it look OK? Bootstrapped and checked on linux-x86_64. Still shows incorrect length attribute computation (described here http://gcc.gnu.org/ml/gcc/2013-07/msg00311.html). Please look at the attached patch that solves length computation problem. The patch also implements length calculation in a generic way, as proposed earlier. The idea is to calculate total insn length via generic length attribute calculation from length_nobnd attribute, but iff length_attribute is non-null. This way, we are able to decorate bnd-prefixed instructions by lenght_nobnd attribute, and generic part will automatically call ix86_bnd_prefixed_insn_p predicate with current insn pattern. I also belive that this approach is most flexible to decorate future patterns. The patch adds new attribute to a couple of patterns to illustrate its usage. Please test this approach. Modulo length calculations, improved by the patch in this message, I have no further comments, but please repost complete (target part) of your patch. Hi Uros, Thanks for your reply! I applied approach you proposed for length attribute. It works well. Make check is clean now. I also adjusted bound registers to recently added mask registers. Attached is a new patch. Thanks, Ilya -- 2013-09-30 Ilya Enkovich ilya.enkov...@intel.com * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-c.c (ix86_target_macros_internal): Add __MPX__. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal): Support MPX ISA. (ix86_conditional_register_usage): Support bound registers. (print_reg): Likewise. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (ix86_print_operand): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (length_nobnd): New. (length): Use length_nobnd if specified. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add bnd prefix and rename length attr to length_nobnd. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On Tue, Sep 17, 2013 at 10:41 AM, Ilya Enkovich enkovich@gmail.com wrote:
The x86 part looks mostly OK (I have a couple of comments bellow), but
please first get target-independent changes reviewed and committed.
Do you mean I should move bound type and mode declaration into a separate
patch?
Yes, target-independent part (middle end) has to go through the
separate review to check if this part is OK. The target-dependent part
uses the infrastructure from the middle end, so it can go into the
code base only after target-independent parts are committed.
I sent a separate patch for bound type and mode class
(http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01268.html). Here is target
part of the patch with fixes you mentioned. Does it look OK?
Bootstrapped and checked on linux-x86_64. Still shows incorrect length
attribute computation (described here
http://gcc.gnu.org/ml/gcc/2013-07/msg00311.html).
Please look at the attached patch that solves length computation
problem. The patch also implements length calculation in a generic
way, as proposed earlier.
The idea is to calculate total insn length via generic length
attribute calculation from length_nobnd attribute, but iff
length_attribute is non-null. This way, we are able to decorate
bnd-prefixed instructions by lenght_nobnd attribute, and generic
part will automatically call ix86_bnd_prefixed_insn_p predicate with
current insn pattern. I also belive that this approach is most
flexible to decorate future patterns.
The patch adds new attribute to a couple of patterns to illustrate its usage.
Please test this approach. Modulo length calculations, improved by the
patch in this message, I have no further comments, but please repost
complete (target part) of your patch.
Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 202953)
+++ config/i386/i386.md (working copy)
@@ -562,12 +562,19 @@
]
(const_int 1)))
+;; When this attribute is set, calculate total insn length from
+;; length_nobnd attribute, prefixed with eventual bnd prefix byte
+(define_attr length_nobnd (const_int 0))
+
;; The (bounding maximum) length of an instruction in bytes.
;; ??? fistp and frndint are in fact fldcw/{fistp,frndint}/fldcw sequences.
;; Later we may want to split them and compute proper length as for
;; other insns.
(define_attr length
- (cond [(eq_attr type other,multi,fistp,frndint)
+ (cond [(eq_attr length_nobnd !0)
+ (plus (symbol_ref (ix86_bnd_prefixed_insn_p (insn)))
+(attr length_nobnd))
+(eq_attr type other,multi,fistp,frndint)
(const_int 16)
(eq_attr type fcmp)
(const_int 4)
@@ -10683,7 +10690,7 @@
%+j%C1\t%l0
[(set_attr type ibr)
(set_attr modrm 0)
- (set (attr length)
+ (set (attr length_nobnd)
(if_then_else (and (ge (minus (match_dup 0) (pc))
(const_int -126))
(lt (minus (match_dup 0) (pc))
@@ -10701,7 +10708,7 @@
%+j%c1\t%l0
[(set_attr type ibr)
(set_attr modrm 0)
- (set (attr length)
+ (set (attr length_nobnd)
(if_then_else (and (ge (minus (match_dup 0) (pc))
(const_int -126))
(lt (minus (match_dup 0) (pc))
@@ -11623,7 +11630,7 @@
[(simple_return)]
reload_completed
ret
- [(set_attr length 1)
+ [(set_attr length_nobnd 1)
(set_attr atom_unit jeu)
(set_attr length_immediate 0)
(set_attr modrm 0)])
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA. 2/2 New registers and instructions
On 16 Sep 11:24, Uros Bizjak wrote: On Fri, Sep 13, 2013 at 12:18 PM, Ilya Enkovich enkovich@gmail.com wrote: 2013/9/11 Uros Bizjak ubiz...@gmail.com: Hi Uros, Thanks a lot for the review! The x86 part looks mostly OK (I have a couple of comments bellow), but please first get target-independent changes reviewed and committed. Do you mean I should move bound type and mode declaration into a separate patch? Yes, target-independent part (middle end) has to go through the separate review to check if this part is OK. The target-dependent part uses the infrastructure from the middle end, so it can go into the code base only after target-independent parts are committed. I sent a separate patch for bound type and mode class (http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01268.html). Here is target part of the patch with fixes you mentioned. Does it look OK? Bootstrapped and checked on linux-x86_64. Still shows incorrect length attribute computation (described here http://gcc.gnu.org/ml/gcc/2013-07/msg00311.html). Thanks, Ilya -- 2013-09-16 Ilya Enkovich ilya.enkov...@intel.com * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-c.c (ix86_target_macros_internal): Add __MPX__. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal): Support MPX ISA. (ix86_conditional_register_usage): Support bound registers. (print_reg): Likewise. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (ix86_print_operand): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. * config/i386/predicates.md (lea_address_operand): Rename to... (address_no_seg_operand): ... this. (address_mpx_no_base_operand): New. (address_mpx_no_index_operand): New. (bnd_mem_operator): New. * config/i386/i386.opt (mmpx): New. * doc/invoke.texi: Add documentation for the flags -mmpx, -mno-mpx. * doc/rtl.texi Add documentation for BND32mode and BND64mode. diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md index 28e626f..79d02f7 100644 --- a/gcc/config/i386/constraints.md +++ b/gcc/config/i386/constraints.md @@ -18,7 +18,7 @@ ;; http://www.gnu.org/licenses/. ;;; Unused letters: -;;; B H T +;;;
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Fri, Sep 13, 2013 at 4:36 PM, H.J. Lu hjl.to...@gmail.com wrote: Did you check the above with x32, where Pmode != word_mode on x86_64? The inner UNSPEC will be generated in SImode, but the matching pattern +(define_insn *mode_mk + [(set (match_operand:BND 0 register_operand =B) +(unspec:BND + [(match_operator:bnd_ptr 3 bnd_mem_operator +[(unspec:bnd_ptr + [(match_operand:bnd_ptr 1 register_operand r) +(match_operand:bnd_ptr 2 address_mpx_no_base_operand Tb)] + UNSPEC_BNDMK_ADDR)])] + UNSPEC_BNDMK))] + TARGET_MPX will have inner UNSPEC in DImode, due to: +;; Bound modes. +(define_mode_iterator BND [(BND32 !TARGET_64BIT) (BND64 TARGET_64BIT)]) + +;; Pointer mode corresponding to bound mode. +(define_mode_attr bnd_ptr [(BND32 SI) (BND64 DI)]) Currently we do not support MPX instrumentation for x32 and therefore I did not check these expands work correctly for x32. I believe the only possible problem here is BND iterator definition which does not care about x32. Following declaration should make everything work fine: Since MPX spec doesn't support x32, MPX should be disallowed with x32. OK, please error out when this invalid combination is detected. Uros.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Fri, Sep 13, 2013 at 12:18 PM, Ilya Enkovich enkovich@gmail.com wrote: 2013/9/11 Uros Bizjak ubiz...@gmail.com: On Tue, Sep 10, 2013 at 1:38 PM, Ilya Enkovich enkovich@gmail.com wrote: Ping^4 Could please someone look at this patch? It is mostly i386 target specific and is basic for further MPX based features. Thanks, Ilya 2013/9/2 Ilya Enkovich enkovich@gmail.com: Ping^3 Attached is the same patch but against the current trunk. 2013/8/26 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/9/11 Uros Bizjak ubiz...@gmail.com: On Tue, Sep 10, 2013 at 1:38 PM, Ilya Enkovich enkovich@gmail.com wrote: Ping^4 Could please someone look at this patch? It is mostly i386 target specific and is basic for further MPX based features. Thanks, Ilya 2013/9/2 Ilya Enkovich enkovich@gmail.com: Ping^3 Attached is the same patch but against the current trunk. 2013/8/26 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Fri, Sep 13, 2013 at 3:18 AM, Ilya Enkovich enkovich@gmail.com wrote: Did you check the above with x32, where Pmode != word_mode on x86_64? The inner UNSPEC will be generated in SImode, but the matching pattern +(define_insn *mode_mk + [(set (match_operand:BND 0 register_operand =B) +(unspec:BND + [(match_operator:bnd_ptr 3 bnd_mem_operator +[(unspec:bnd_ptr + [(match_operand:bnd_ptr 1 register_operand r) +(match_operand:bnd_ptr 2 address_mpx_no_base_operand Tb)] + UNSPEC_BNDMK_ADDR)])] + UNSPEC_BNDMK))] + TARGET_MPX will have inner UNSPEC in DImode, due to: +;; Bound modes. +(define_mode_iterator BND [(BND32 !TARGET_64BIT) (BND64 TARGET_64BIT)]) + +;; Pointer mode corresponding to bound mode. +(define_mode_attr bnd_ptr [(BND32 SI) (BND64 DI)]) Currently we do not support MPX instrumentation for x32 and therefore I did not check these expands work correctly for x32. I believe the only possible problem here is BND iterator definition which does not care about x32. Following declaration should make everything work fine: Since MPX spec doesn't support x32, MPX should be disallowed with x32. -- H.J.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Tue, Sep 10, 2013 at 1:38 PM, Ilya Enkovich enkovich@gmail.com wrote: Ping^4 Could please someone look at this patch? It is mostly i386 target specific and is basic for further MPX based features. Thanks, Ilya 2013/9/2 Ilya Enkovich enkovich@gmail.com: Ping^3 Attached is the same patch but against the current trunk. 2013/8/26 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
Ping^4 Could please someone look at this patch? It is mostly i386 target specific and is basic for further MPX based features. Thanks, Ilya 2013/9/2 Ilya Enkovich enkovich@gmail.com: Ping^3 Attached is the same patch but against the current trunk. 2013/8/26 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. *
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
Ping 2013/8/19 Ilya Enkovich enkovich@gmail.com: Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. * config/i386/predicates.md (lea_address_operand): Rename to... (address_no_seg_operand): ... this. (address_mpx_no_base_operand): New. (address_mpx_no_index_operand): New. (bnd_mem_operator): New. * config/i386/i386.opt (mmpx): New. * doc/invoke.texi: Add documentation for the
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
Ping 2013/8/12 Ilya Enkovich enkovich@gmail.com: 2013/8/10 Joseph S. Myers jos...@codesourcery.com: On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com Thanks! Here is a new revision with -mmpx and new machine modes / class documented. Is it good to install to trunk? Thanks, Ilya --- 2013-08-12 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. * config/i386/predicates.md (lea_address_operand): Rename to... (address_no_seg_operand): ... this. (address_mpx_no_base_operand): New. (address_mpx_no_index_operand): New. (bnd_mem_operator): New. * config/i386/i386.opt (mmpx): New. * doc/invoke.texi: Add documentation for the flags -mmpx, -mno-mpx. * doc/rtl.texi
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/8/8 Joseph S. Myers jos...@codesourcery.com: On Thu, 8 Aug 2013, Ilya Enkovich wrote: That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I just realized that usage of option which is already defined for other languages may be problematic when this option is passed to MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers? Seems reasonable to me. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? I'm not sure why the libgfortran build would have failed ... maybe some libraries don't in fact do anything with pointers for which the checks would help, but if so then I'd expect the option simply not to have any effect on the code generated for those libraries. Multilibs are expected to be the same for all libraries (but packagers could no doubt optimize things in their packages, if in fact some libraries are identical when built both with and without MPX). The reason for libgfortran build was trivial - -fmpx usage combined with -Werror. This is easily fixed and seems the only problem now is to allow legacy library usage with MPX binary (see http://gcc.gnu.org/ml/gcc/2013-08/msg00114.html). Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Mon, 29 Jul 2013, Ilya Enkovich wrote: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). I think the -mmpx option should be documented in invoke.texi, and the new machine modes / mode class should be documented in rtl.texi where other machine modes / mode classes are documented. Beyond that, I have no comments on this patch revision. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/8/8 Joseph S. Myers jos...@codesourcery.com: On Fri, 2 Aug 2013, Ilya Enkovich wrote: Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. Thanks. As noted, there should be a clean separation between what's generic and what's architecture-specific - the generic command-line options, hooks etc. shouldn't mention MPX in their names. That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I'm unclear on the references to *_nobnd and *_nochk functions - are there corresponding library (glibc etc.) changes to add additional function variants? Are all built-in functions that use pointers modified so that GCC will insert the required checks when expanding inline? The idea was to add new versions of some functions into glibc and replace normal versions call with special versions call when possible (e.g. when we know memcpy call does not copy pointers). Currently in MPX mode expanding is allowed for _nochk_nobnd variant which is equal to regular call. I cannot be sure new version will be added into glibc. If not, we would probably create additional library in GCC. I'm also unclear on how much --enable-mpx does - in general, it's desirable for a single compiler to be able to generate binaries both with and without the checks, and so quite possibly to build libgcc, libstdc++ etc. as multilibs both with and without MPX, rather than needing to make a static decision when GCC is built (so, any configure option should preferably be about building *extra* library variants, for example, rather than changing the options with which existing variants are built). Currently (in public mpx branch) --enable-mpx just adds compilation options to some libraries. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/8/8 Ilya Enkovich enkovich@gmail.com: 2013/8/8 Joseph S. Myers jos...@codesourcery.com: On Fri, 2 Aug 2013, Ilya Enkovich wrote: Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. Thanks. As noted, there should be a clean separation between what's generic and what's architecture-specific - the generic command-line options, hooks etc. shouldn't mention MPX in their names. That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I just realized that usage of option which is already defined for other languages may be problematic when this option is passed to MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers? Thanks, Ilya I'm unclear on the references to *_nobnd and *_nochk functions - are there corresponding library (glibc etc.) changes to add additional function variants? Are all built-in functions that use pointers modified so that GCC will insert the required checks when expanding inline? The idea was to add new versions of some functions into glibc and replace normal versions call with special versions call when possible (e.g. when we know memcpy call does not copy pointers). Currently in MPX mode expanding is allowed for _nochk_nobnd variant which is equal to regular call. I cannot be sure new version will be added into glibc. If not, we would probably create additional library in GCC. I'm also unclear on how much --enable-mpx does - in general, it's desirable for a single compiler to be able to generate binaries both with and without the checks, and so quite possibly to build libgcc, libstdc++ etc. as multilibs both with and without MPX, rather than needing to make a static decision when GCC is built (so, any configure option should preferably be about building *extra* library variants, for example, rather than changing the options with which existing variants are built). Currently (in public mpx branch) --enable-mpx just adds compilation options to some libraries. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Thu, 8 Aug 2013, Ilya Enkovich wrote: That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I just realized that usage of option which is already defined for other languages may be problematic when this option is passed to MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers? Seems reasonable to me. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? I'm not sure why the libgfortran build would have failed ... maybe some libraries don't in fact do anything with pointers for which the checks would help, but if so then I'd expect the option simply not to have any effect on the code generated for those libraries. Multilibs are expected to be the same for all libraries (but packagers could no doubt optimize things in their packages, if in fact some libraries are identical when built both with and without MPX). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Fri, 2 Aug 2013, Ilya Enkovich wrote: Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. Thanks. As noted, there should be a clean separation between what's generic and what's architecture-specific - the generic command-line options, hooks etc. shouldn't mention MPX in their names. I'm unclear on the references to *_nobnd and *_nochk functions - are there corresponding library (glibc etc.) changes to add additional function variants? Are all built-in functions that use pointers modified so that GCC will insert the required checks when expanding inline? I'm also unclear on how much --enable-mpx does - in general, it's desirable for a single compiler to be able to generate binaries both with and without the checks, and so quite possibly to build libgcc, libstdc++ etc. as multilibs both with and without MPX, rather than needing to make a static decision when GCC is built (so, any configure option should preferably be about building *extra* library variants, for example, rather than changing the options with which existing variants are built). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. What about the first patch? Should I post next patches in the series? Thanks, Ilya 2013/7/29 Ilya Enkovich enkovich@gmail.com: Hi, Here is updated version of the patch. I removed redundant mode_for_bound, added comments to BOUND_TYPE and added -mmpx option. I also fixed bndmk/bndldx/bndstx constraints to avoid incorrect register allocation (created two new constraints for that). Thanks, Ilya --- 2013-07-29 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. * config/i386/constraints.md (B): New. (Ti): New. (Tb): New. * config/i386/i386-modes.def (BND32): New. (BND64): New. * config/i386/i386-protos.h (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.c (isa_opts): Add mmpx. (regclass_map): Add bound registers. (dbx_register_map): Likewise. (dbx64_register_map): Likewise. (svr4_dbx_register_map): Likewise. (PTA_MPX): New. (ix86_option_override_internal) Support MPX ISA. (ix86_code_end): Add MPX bnd prefix. (output_set_got): Likewise. (ix86_output_call_insn): Likewise. (get_some_local_dynamic_name): Add '!' (MPX bnd) print prefix support. (ix86_print_operand_punct_valid_p): Likewise. (ix86_print_operand_address): Support UNSPEC_BNDMK_ADDR and UNSPEC_BNDMK_ADDR. (ix86_class_likely_spilled_p): Add bound regs support. (ix86_hard_regno_mode_ok): Likewise. (x86_order_regs_for_local_alloc): Likewise. (ix86_bnd_prefixed_insn_p): New. * config/i386/i386.h (FIRST_PSEUDO_REGISTER): Fix to new value. (FIXED_REGISTERS): Add bound registers. (CALL_USED_REGISTERS): Likewise. (REG_ALLOC_ORDER): Likewise. (HARD_REGNO_NREGS): Likewise. (TARGET_MPX): New. (VALID_BND_REG_MODE): New. (FIRST_BND_REG): New. (LAST_BND_REG): New. (reg_class): Add BND_REGS. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BND_REGNO_P): New. (ANY_BND_REG_P): New. (BNDmode): New. (HI_REGISTER_NAMES): Add bound registers. * config/i386/i386.md (UNSPEC_BNDMK): New. (UNSPEC_BNDMK_ADDR): New. (UNSPEC_BNDSTX): New. (UNSPEC_BNDLDX): New. (UNSPEC_BNDLDX_ADDR): New. (UNSPEC_BNDCL): New. (UNSPEC_BNDCU): New. (UNSPEC_BNDCN): New. (UNSPEC_MPX_FENCE): New. (BND0_REG): New. (BND1_REG): New. (type): Add mpxmov, mpxmk, mpxchk, mpxld, mpxst. (length_immediate): Likewise. (prefix_0f): Likewise. (memory): Likewise. (prefix_rep): Check for bnd prefix. (BND): New. (bnd_ptr): New. (BNDCHECK): New. (bndcheck): New. (*jcc_1): Add MPX bnd prefix and fix length. (*jcc_2): Likewise. (jump): Likewise. (simple_return_internal): Likewise. (simple_return_pop_internal): Likewise. (*indirect_jump): Add MPX bnd prefix. (*tablejump_1): Likewise. (simple_return_internal_long): Likewise. (simple_return_indirect_internal): Likewise. (mode_mk): New. (*mode_mk): New. (movmode): New. (*movmode_internal_mpx): New. (mode_bndcheck): New. (*mode_bndcheck): New. (mode_ldx): New. (*mode_ldx): New. (mode_stx): New. (*mode_stx): New. * config/i386/predicates.md (lea_address_operand) Rename to... (address_no_seg_operand): ... this. (address_mpx_no_base_operand): New. (address_mpx_no_index_operand): New. (bnd_mem_operator): New. * config/i386/i386.opt (mmpx): New.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/7/25 Joseph S. Myers jos...@codesourcery.com: On Wed, 24 Jul 2013, Ilya Enkovich wrote: Well, this patch does not introduce any changes on user-visible level. It just adds MPX instructions support to i386 target. Usually each new x86 instruction has corresponding builtin function and therefore is provided with a testcase. But MPX instructions do not have builtin function for direct generation and therefore there are no tests for this patch. Usually also new instructions have a -m option to enable them, but you don't have that here either. I realise the instructions are NOPs on processors not supporting them (all processors not supporting them?), but given that the availability of the instructions must at some point affect either the semantics of RTL containing checks, or what RTL the compiler can generate code for, I'd think you should have such an option anyway. We had this option initially but removed it because it was useless. You get nothing when pass -mmpx and you always have to to pass it when pass -fmpx. Introduction of that option just makes you to always use pair '-fmpx -mmpx' which is not convenient. I have subsequent patches to add user-visible features based on MPX. Sorry for missing series indicator (fixed). High-level feature based on MPX is described on GCC Wiki (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). Thanks. You also need to post the implementation design in terms of semantics of machine-independent tree/GIMPLE/RTL additions (which of course should be added to the relevant .texi files). Could you please be more specific regarding .texi files? In my implementation I add - new type - new modes - new options - new builtins - new target hooks I know where hooks and options should be documented. But what about others? For now I saw only description in comments. tree-mpx.c files has a description in a header. Does it look close to implementation design you are talking about? Having options starting -fmpx seems odd. If something is machine-independent, as using -f options implies, then the options should have a machine-independent name rather than being named after one particular implementation. Given we already have a -fbounds-check option (documented as supported for Java and Fortran only), that seems an obvious possibility for naming. If for some reason it's a bad idea to use that option, at least the option names should make clear the similarities and differences from that option. More generally, the patch series submission should include a comparison with other bounds-checking features in GCC such as -fmudflap and -fsanitize=address (and the user documentation should enable users to understand the advantages and disadvantages of each, so they can choose which is the best match to their requirements). I'm far from clear at present about why so many different approaches are needed, at the user-interface level, and whether there is anything in common between them that could usefully be shared within the implementations in GCC. I'll look at -fbounds-check option. I'll also add some additional description and comparison on GCC Wiki. As I understand it, the feature logically splits into: * Support the bounds checking within functions but with bounds being lost for function arguments and return values. This does not need any architecture-specific support, although it would be slow without such support. * Bounds checking across ABI boundaries. This requires the architecture support (although one could also imagine such a feature defined in a way not needing new registers / instructions - again, slow). So, if the user requests this and doesn't use -mmpx to enable the instructions, an error would seem appropriate. Maybe the first is of too little value without the hardware support (e.g. too slow), and so the two only make sense together - I don't know. But in any case it needs to be clear what the machine-independent semantics of each option are, and what does or does not depend in hardware support. Do you do anything to disable optimizations that are based on making deductions from assuming that a pointer is valid? I've never tried to look at it in such way. I also doubt any of this parts is useful without the other one. Current instrumentation pass is target independent. Implementation allows target dependent support (via implementation of hooks in a target; currently done on i386 only) and general support (via implementation of generic builtin functions and default hooks; currently not implemented). Currently we disable ivopts because we it looses base pointer for memory reference (full optimization disabling is not required here and we are going to turn it on back when find a nice fix). We also have problems with loosing field accesses. I do not think we've found all cases when optimization leads to uncaught bound violations or false bound violations. But we work in
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Thu, 25 Jul 2013, Ilya Enkovich wrote: Usually also new instructions have a -m option to enable them, but you don't have that here either. I realise the instructions are NOPs on processors not supporting them (all processors not supporting them?), but given that the availability of the instructions must at some point affect either the semantics of RTL containing checks, or what RTL the compiler can generate code for, I'd think you should have such an option anyway. We had this option initially but removed it because it was useless. You get nothing when pass -mmpx and you always have to to pass it when pass -fmpx. Introduction of that option just makes you to always use pair '-fmpx -mmpx' which is not convenient. It still seems wrong for -f options to enable new architecture-specific instructions; that's the job of -m options (including -march= options that imply -mmpx, of course). Thanks. You also need to post the implementation design in terms of semantics of machine-independent tree/GIMPLE/RTL additions (which of course should be added to the relevant .texi files). Could you please be more specific regarding .texi files? In my implementation I add - new type - new modes - new options - new builtins - new target hooks I know where hooks and options should be documented. But what about others? For now I saw only description in comments. Built-in functions are documented in extend.texi. Some aspects of tree and GIMPLE are documented in generic.texi and gimple.texi. The various individual machine modes, and classes of modes, are documented in rtl.texi. If there are new RTL insn patterns, those are also documented in rtl.texi. Configure options are documented in install.texi. (Arguably, if MPX-enabled versions of runtime libraries are built they should be *in addition to* normal versions - that is, extra multilibs, as the right choice may not be the same for all programs built with a given compiler.) tree-mpx.c files has a description in a header. Does it look close to implementation design you are talking about? It seems useful - but to the extent that it refers specifically to MPX (or the file is so-named to refer to MPX) it's at the wrong level. It should describe things in terms of the architecture-independent GIMPLE representation of the bounds checks (with any architecture-dependencies needed at this point, controlled by hooks, being subsidiary to that). Then, I suppose there would be lowering to some architecture-specific form, whether as a GIMPLE pass or at the time of RTL expansion. I'll look at -fbounds-check option. I'll also add some additional description and comparison on GCC Wiki. Thanks. I'd imagine the desired state as: * generic infrastructure for determining bounds for objects, tracking them through pointer arithmetic, optimizing away redundant bounds checks, etc., which could be shared between multiple different forms of bounds checking supported in GCC; and * implementations of the different bounds-checking strategies (MPX, ASAN, etc.), based on the information / internal representation built up by common infrastructure, and using architecture-specific features where appropriate. Maybe that's not realistic given the present state of GCC or the differences between the different bounds-checking approaches. But you're the one working on the bounds checking - you need to produce the explanation of what there is or is not in common between the different approaches that justifies the choice of what to share or not to share in the implementations. And you also need to write the user documentation for invoke.texi that allows a user to understand the trade-offs at the level of their source code, the performance of the generated code and what conditions will or will not be detected, so users can make sensible choices of which bounds-checking options to use based on their requirements. Naming new built-in functions __bnd_* seems unfortunate. We already have __builtin_*, __sync_* and __atomic_* as built-in function naming conventions (with consequent checks in various places based on function name) - do we really need yet another such prefix? Both __bnd_ and __builtin___bnd variants are available. The reason for builtins name is simple - Intel docs. It is always nice to have compatibility. Actually Intel declares single underscore in the beginning of the name but I could not make it work on C++ with a single underscore. Is there a way to fix it? The normal way to have compatibility in such cases is a header that #defines the other-compiler names to the GCC names. At least is_builtin_name would need updating for any new built-in function name prefix - but as noted, I don't advise adding such new prefixes at all. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/7/24 Joseph S. Myers jos...@codesourcery.com: On Wed, 24 Jul 2013, Ilya Enkovich wrote: Here is a patch which adds support for new instructions from Intel Memory Protection Extensions (MPX) ISA [1] This patch introduces bound type, modes, registers and all MPX instructions. Control transfer instructions are modified to support 'bnd' prefix used by Intel MPX. Bootstrap and checked on x86_64-linux. There is one issue [2] with length attribute computation which leads to several fails in make check. Currently I assume it is not a bug in this patch. But in case it is a patch problem I have a version which uses temp attribute for length computation as a workaround (resulting in clean make check). Does it look good for trunk? This patch has no documentation (no changes to .texi files to document the new features added for users compiling code in C or other languages) and no testcases, so I have no idea what it is supposed to do at the user-visible level and I don't think it's suitable to be considered for trunk without either adding documentation and testcases to it, or posting subsequent patches in the series that actually add the user-visible features (and include documentation and testcases for them) that this patch is meant to enable. (If this is the start of a patch series, you probably want to start with a 0/N mail from that series, explaining the processor feature in high-level terms, how you intend that feature to be used from C code, the internal datastructures used inside the compiler to correspond to the uses of that feature from C, and the rationale for the choices you made. Then this present patch might be 1/N, followed by a series of other patches adding other infrastructure or user-visible features.) Well, this patch does not introduce any changes on user-visible level. It just adds MPX instructions support to i386 target. Usually each new x86 instruction has corresponding builtin function and therefore is provided with a testcase. But MPX instructions do not have builtin function for direct generation and therefore there are no tests for this patch. I have subsequent patches to add user-visible features based on MPX. Sorry for missing series indicator (fixed). High-level feature based on MPX is described on GCC Wiki (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). Current implementation state is available in recently announced mpx branch. Your new function mode_for_bound lacks a comment explaining its semantics (those of the arguments and return value). The definition of the new tree code BOUND_TYPE needs a substantial comment explaining the full, machine-independent semantics of such a tree type and of whatever tree fields are used in such a type - it's not a standard C concept. Thanks for comments! I'll add descriptions. Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Wed, 24 Jul 2013, Ilya Enkovich wrote: Well, this patch does not introduce any changes on user-visible level. It just adds MPX instructions support to i386 target. Usually each new x86 instruction has corresponding builtin function and therefore is provided with a testcase. But MPX instructions do not have builtin function for direct generation and therefore there are no tests for this patch. Usually also new instructions have a -m option to enable them, but you don't have that here either. I realise the instructions are NOPs on processors not supporting them (all processors not supporting them?), but given that the availability of the instructions must at some point affect either the semantics of RTL containing checks, or what RTL the compiler can generate code for, I'd think you should have such an option anyway. I have subsequent patches to add user-visible features based on MPX. Sorry for missing series indicator (fixed). High-level feature based on MPX is described on GCC Wiki (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). Thanks. You also need to post the implementation design in terms of semantics of machine-independent tree/GIMPLE/RTL additions (which of course should be added to the relevant .texi files). Having options starting -fmpx seems odd. If something is machine-independent, as using -f options implies, then the options should have a machine-independent name rather than being named after one particular implementation. Given we already have a -fbounds-check option (documented as supported for Java and Fortran only), that seems an obvious possibility for naming. If for some reason it's a bad idea to use that option, at least the option names should make clear the similarities and differences from that option. More generally, the patch series submission should include a comparison with other bounds-checking features in GCC such as -fmudflap and -fsanitize=address (and the user documentation should enable users to understand the advantages and disadvantages of each, so they can choose which is the best match to their requirements). I'm far from clear at present about why so many different approaches are needed, at the user-interface level, and whether there is anything in common between them that could usefully be shared within the implementations in GCC. As I understand it, the feature logically splits into: * Support the bounds checking within functions but with bounds being lost for function arguments and return values. This does not need any architecture-specific support, although it would be slow without such support. * Bounds checking across ABI boundaries. This requires the architecture support (although one could also imagine such a feature defined in a way not needing new registers / instructions - again, slow). So, if the user requests this and doesn't use -mmpx to enable the instructions, an error would seem appropriate. Maybe the first is of too little value without the hardware support (e.g. too slow), and so the two only make sense together - I don't know. But in any case it needs to be clear what the machine-independent semantics of each option are, and what does or does not depend in hardware support. Do you do anything to disable optimizations that are based on making deductions from assuming that a pointer is valid? Naming new built-in functions __bnd_* seems unfortunate. We already have __builtin_*, __sync_* and __atomic_* as built-in function naming conventions (with consequent checks in various places based on function name) - do we really need yet another such prefix? -- Joseph S. Myers jos...@codesourcery.com
