public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] tree-optimization/109609 - correctly interpret arg size in fnspec
Date: Tue, 25 Apr 2023 15:26:16 +0200	[thread overview]
Message-ID: <ZEfU+ASkUGrG2qjK@kam.mff.cuni.cz> (raw)
In-Reply-To: <20230425130836.5E2B513466@imap2.suse-dmz.suse.de>

> By majority vote and a hint from the API name which is
> arg_max_access_size_given_by_arg_p this interprets a memory access
> size specified as given as other argument such as for strncpy
> in the testcase which has "1cO313" as specifying the _maximum_
> size read/written rather than the exact size.  There are two
> uses interpreting it that way already and one differing.  The
> following adjusts the differing and clarifies the documentation.

This makes sense to me. Bit sad is memcpy/memset may be common enough so
the extra disambiguation based on maximal object size may be useful.
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> 
> It looks like arg_access_size_given_by_type_p is interpreted in
> a similar way but the API doesn't reflect this (no _max).  On IRC
> we discussed it would be nice to have both, exact and non-exact
> size (for example use 'a'..'j' for the alternate variant).

Hmm 'a'...'j' may work to stick this information in....
> 
> I still wonder if my interpretation is correct in that '1'..'9'
> specify a _bound_ for the size.  Honza, do you remember why
> you wrote the IPA modref users this way?  Was that intentional?
> Thus is my reverse engineering of the desired semantics correct?

My fnspec implementation was based on what the code was doing
beforehand. In GCC 10 we have:

        case BUILT_IN_STRCPY:
        case BUILT_IN_STRNCPY:
        case BUILT_IN_MEMCPY:
        case BUILT_IN_MEMMOVE:
        case BUILT_IN_MEMPCPY:
        case BUILT_IN_STPCPY:
        case BUILT_IN_STPNCPY:
        case BUILT_IN_TM_MEMCPY:
        case BUILT_IN_TM_MEMMOVE:
          {
            ao_ref dref;
            tree size = NULL_TREE;
            if (gimple_call_num_args (call) == 3)
              size = gimple_call_arg (call, 2);
            ao_ref_init_from_ptr_and_size (&dref,
                                           gimple_call_arg (call, 1),
                                           size);
            return refs_may_alias_p_1 (&dref, ref, false);
          }
So it seem that we handled strncpy the way claiming full size access
there too and I simply copied it into the new implementation.

While adding fnspec I just used adapted this code, so the difference
between modref and tree-ssa-alias implementation was not quite
intentional.
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/109609
> 	* attr-fnspec.h (arg_max_access_size_given_by_arg_p):
> 	Clarify semantics.
> 	* tree-ssa-alias.cc (check_fnspec): Correctly interpret
> 	the size given by arg_max_access_size_given_by_arg_p as
> 	maximum, not exact, size.
> 
> 	* gcc.dg/torture/pr109609.c: New testcase.
Patch looks good to me (modulo the fact that it would be nice to be also
able to handle memcpy/memset precisely).

Honza

      reply	other threads:[~2023-04-25 13:26 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25 13:08 Richard Biener
2023-04-25 13:26 ` Jan Hubicka [this message]

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=ZEfU+ASkUGrG2qjK@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).