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
