Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2013/7/24 Joseph S. Myers <joseph@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
> joseph@codesourcery.com