This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Uros Bizjak <ubizjak at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>, Areg Melik-Adamyan <areg dot melikadamyan at gmail dot com>
- Date: Thu, 25 Jul 2013 14:37:19 +0000
- Subject: Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
- References: <CAMbmDYZdrR8t66VtNPFxbz33mMX0M-tMj_Bv=uvrejui_YnVWg at mail dot gmail dot com> <Pine dot LNX dot 4 dot 64 dot 1307242237420 dot 5307 at digraph dot polyomino dot org dot uk> <CAMbmDYbNvPh8MiJkgx29Hc+d_czzLeFuxeG4uDz_AozqX3j1cg at mail dot gmail dot com>
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
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
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
> > 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
Joseph S. Myers