Re: [PATCH, i386, Pointer Bounds Checker 32/x] Pointer Bounds Checker hooks for i386 target

[Uros Bizjak <ubizjak at gmail dot com>]

On Tue, Sep 23, 2014 at 8:48 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2014-09-22 22:51 GMT+04:00 Uros Bizjak <ubizjak@gmail.com>:
>> On Mon, Sep 22, 2014 at 5:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> On 19 Sep 18:21, Uros Bizjak wrote:
>>>> On Fri, Sep 19, 2014 at 2:53 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>
>>>> >> > This patch adds i386 target hooks for Pointer Bounds Checker.
>>>>
>>>> > New version with fixes and better documentation for ix86_load_bounds and ix86_store_bounds is below.
>>>>
>>>> > +/* Expand pass uses this hook to load bounds for function parameter
>>>> > +   PTR passed in SLOT in case its bounds are not passed in a register.
>>>> > +
>>>> > +   If SLOT is a memory, then bounds are loaded as for regular pointer
>>>> > +   loaded from memory.  PTR may be NULL in case SLOT is a memory.
>>>> > +   In such case value of PTR (if required) may be loaded from SLOT.
>>>> > +
>>>> > +   If SLOT is NULL or a register then SLOT_NO is an integer constant
>>>> > +   holding number of the target dependent special slot which should be
>>>> > +   used to obtain bounds.
>>>> > +
>>>> > +   Return loaded bounds.  */
>>>>
>>>> OK, I hope I understand this target-handling of SLOT_NO. Can you
>>>> please clarify when SLOT is a register?
>>>
>>> For functions with more than four pointers passed in registers we do not have enough bound registers to pass bounds.  These hooks are called then with SLOT set to register used to pass pointer
>>>
>>>>
>>>> I propose to write this function in the following (hopefully equivalent) way:
>>>
>>> Since addr computation is very similar for both loading and storing (the only difference is usage of either arg_pointer_rtx or stack_pointer_rtx) I decided additionally move it into a separate function.  This should make functions simplier for understanding.
>>
>> LGTM, just add the explanation when NULL is returned.
>
> There is no path in this function returning NULL (we are talking about
> ix86_get_arg_address_for_bt, right?).

Oh, in fact, I was looking at ix86_function_value_bounds which doesn't
have comment at all... Looking a bit more, there are some other
functions without comments

Uros.