2013/11/7 Jeff Law <l...@redhat.com>:
> On 11/04/13 05:40, Richard Biener wrote:
>>>
>>>
>>> Effectively the bounds are passed "on the side".
>>
>>
>> Well, not exactly.  I can see __bound_tmp.0_4 passed to access_and_store.
>
> I'm referring to how they're dealt with in FUNCTION_ARG and friends, ie, the
> low level aspects.  Maybe that's why we're crossing wires here.
>
>
>
>>
>> Are __builtin_ia32_bndcu and access_and_store supposed to be
>> called directly after each other?  What if for example profile
>> instrumentation
>> inserts a call inbetween them?
>
> That's a question for Ilya.

I think there is some misunderstanding. __builtin_ia32_bndcu and
access_and_store calls are completely independent in this example.
This is just a code from example on how user-level builtins may be
used with legacy code.

>
>
>>>
>>> You can't actually interleave them -- that results in MPX and normal code
>>> not being able to interact.   Passing the bound at the end doesn't really
>>> work either -- varargs and the desire to pass some of the bounds around
>>> in
>>> bound registers.
>>
>>
>> I'm only looking at how bound arguments are passed at the GIMPLE
>> level - which I think is arbitrary given they are passed on-the-side
>> during code-generation.  So I'm looking for a more "sensible" option
>> for the GIMPLE representation which looks somewhat fragile to me
>> (it looks the argument is only there to have a data dependence
>> between the bndcu intrinsic and the actual call?)
>
> OK, we're clearly looking at two different aspects here.  While I think we
> can express them arbitrarily in GIMPLE, we have to pass them on the side
> once we get into the low level code for compatibility purposes.
>
> I think argument passing for MPX is going to ultimately be ugly/fragile
> regardless of what we do given the constraints.  Given we're passing on the
> side at the low level, I'd prefer them passed on the side in GIMPLE, but I'm
> willing to consider other alternatives.
>
>
>>
>>>>
>>>> /* Return the number of arguments used by call statement GS
>>>>      ignoring bound ones.  */
>>>>
>>>> static inline unsigned
>>>> gimple_call_num_nobnd_args (const_gimple gs)
>>>> {
>>>>     unsigned num_args = gimple_call_num_args (gs);
>>>>     unsigned res = num_args;
>>>>     for (unsigned n = 0; n < num_args; n++)
>>>>       if (POINTER_BOUNDS_P (gimple_call_arg (gs, n)))
>>>>         res--;
>>>>     return res;
>>>> }
>>>>
>>>> the choice means that gimple_call_num_nobnd_args is not O(1).
>>>
>>>
>>> Yes, but I don't see that's terribly problematical.
>>
>>
>> Well, consider compile.exp limits-fnargs.c written with int * parameters
>> and bound support ;)  [there is a limit on the number of bound registers
>> available, but the GIMPLE parts doesn't put any limit on instrumented
>> args?]
>
> Right.  Go back to Ilya's earlier messages :-)
>
> There's these two magic hooks TARGET_{LOAD,STORE}_BOUNDS_FOR_ARG.  At some
> point when you run out of regs, you start shoving these things into memory.
> I think we're ultimately going to need further clarification here.
>
>
>>>>
>>>> GIMPLE layout depending on flag_check_pointer_bounds sounds
>>>> like a recipie for desaster if you consider TUs compiled with and
>>>> TUs compiled without and LTO.  Or if you consider using
>>>> optimized attribute with that flag.
>>>
>>>
>>> Sorry, I don't follow.  Can you elaborate please.
>>
>>
>> Compile TU1 with -fno-chk, TU2 with -fchk, LTO both object files
>> which gives you - well, either -fno-chk or -fchk.  Then you read in
>> the GIMPLE and when calling gimple_call_nobnd_arg you get
>> a mismatch between where you generated the gimple and where
>> you use the gimple.
>>
>> A flag on whether a call is instrumented is better (consider inlining
>> from TU1 into TU2 where a flag on struct function for example wouldn't
>> be enough either).
>
> OK.  I see what you're saying.  Whether or not we have these extra args is a
> property of the call site, not the CFUN, target DECL or the TU. That makes
> sense.
>
>
> Ilya, I think there's enough confusion & issues around the call ABI/API that
> we need to sort it out before moving forward in any significant way.
>
> Let's go back to argument passing and go through some concrete examples of
> the various cases.  Make sure to tie them into the calls to
> TARGET_{LOAD,STORE}_BOUNDS_FOR_ARG and the new gimple_call_* routines.

OK. Lets see at how expand pass handles instrumented calls.  As usual,
for each passed argument expand calls target hook to determine where
argument is passed.  Now target may return not only slot for arg, but
also a slot for bounds in case arg may have them (PARALLEL expr is
used to hold all slots).  If bounds slot is register, then expand
simply puts bounds to it.  If it is not a register, target hook is
called to store bounds (TARGET_STORE_BOUNDS_FOR_ARG).  Incoming bounds
are handled in a similar way but with usage of
TARGET_LOAD_BOUNDS_FOR_ARG hook.

The only remained problem for expand here is to identify which bounds
should be passed for arg.  To tie bounds to argument I put all bounds
passed for arg (in case arg is a structure, we may have multiple
bounds) as additional arguments in the call.  Each regular argument is
immediately followed by all bounds passed for it. For example:

test (&"Hello world!"[0], 0);

is translated into

test (&"Hello world!"[0], __bound_tmp.0_1, 0);

where __bounds_tmp.0_1 - bounds of passed string.

Of course, such call modification has some implications.  Some
optimizations may rely on number of arguments in the call (e,g, strlen
pass) and indexes of arguments.  Also now index of arg in fndecl does
not match the index of actual arg in the call.  For such cases new
functions are introduced to get args by it's original index, like
there is no instrumentation.  BTW there are not many usages of these
new functions and almost all of them are in strlen pass.

Also some changes, like the patch I sent for calls modifications and
copy, are required.  I suppose such changes are quite simple. and not
very wide.

Regarding proposal to put all bounds to the end of call's arg list - I
do not think it makes life much easier.  It still requires
modifications in verifier (because it checks arg count), calls copy
etc.  It becomes more complex to identify bounds of arg.  Also GIMPLE
dump for calls becomes less informative.  Surely it allows to get rid
of gimple_call_nobnd_arg and gimple_call_get_nobnd_arg_index but will
require additional interface functions for bound args.

Ilya

>
> Jeff

Reply via email to