public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Ilya Enkovich <enkovich.gnu@gmail.com>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [CHKP] Support returned bounds in thunks expand
Date: Fri, 03 Apr 2015 08:46:00 -0000	[thread overview]
Message-ID: <CAMbmDYY8yYNoCy4hX_zBdKMcg1j=f9nwS_nXdUimUPFdm_mUMA@mail.gmail.com> (raw)
In-Reply-To: <20150402205551.GJ21276@atrey.karlin.mff.cuni.cz>

2015-04-02 23:55 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>:
>> Ping
>>
>> 2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> > Hi,
>> >
>> > Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>> > 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>> >         bounds for instrumented functions.
>
> I think if the extra bultin call is needed here, the same andling needs to be
> added to ipa-split.  We really ought to unify that code - it is surprisingly
> difficult to produce a wrapper call these days.

Yep, there is such code in place. It is done by
insert_bndret_call_after. You are right, I should use the same
interface for both these cases.

>> > +         if (instrumentation_clone
>> > +             && !DECL_BY_REFERENCE (resdecl)
>> > +             && restmp
>> > +             && BOUNDED_P (restmp))
>> > +           {
>> > +             tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
>> > +             gcall *retbnd = gimple_build_call (fn, 1, restmp);
>> > +
>> > +             resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
>> > +             gimple_call_set_lhs (retbnd, resbnd);
>> > +
>> > +             gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
>> > +             create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
>> > +           }
>
> I am not sure why we need to check here:  Originaly we have two bounded functions, A and B.
> We turn function B to a wrapper of function A. If callers of bounded functions need
> to call a builtin, I would expect all callers of B to do it already, so I would expect
> that wrapper is safe here?

The problem with instrumented call is that instrumented function
returns two values and call lhs gets only the first one. Thus we
generate bndret call to get the second one to build own return with
two values.

>
> Or do we mix instrumented and non-instrumented versions somehow?
>
> Addding code handling return value is going to turn the call to non-tailcall, so you probably
> want to drop the tailcall flag, too.

No, function still return what the last call return, thus may keep tailcall.

Thanks,
Ilya

>
> Honza

  reply	other threads:[~2015-04-03  8:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10 10:13 Ilya Enkovich
2015-04-02 14:49 ` Ilya Enkovich
2015-04-02 19:42   ` Jeff Law
2015-04-03  8:38     ` Ilya Enkovich
2015-04-02 20:55   ` Jan Hubicka
2015-04-03  8:46     ` Ilya Enkovich [this message]
2015-04-03 17:05       ` Jan Hubicka
2015-04-07 14:12         ` Ilya Enkovich
2015-04-07 20:33           ` Jan Hubicka
2015-04-07 23:28             ` Ilya Enkovich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMbmDYY8yYNoCy4hX_zBdKMcg1j=f9nwS_nXdUimUPFdm_mUMA@mail.gmail.com' \
    --to=enkovich.gnu@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).