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
