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

["Joseph S. Myers" <joseph at codesourcery dot 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.

> 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
joseph@codesourcery.com