public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Martin Sebor <msebor@gmail.com>
Cc: "Martin Liška" <mliska@suse.cz>,
	"GCC Patches" <gcc-patches@gcc.gnu.org>,
	"Richard Biener" <richard.guenther@gmail.com>,
	"Jan Hubicka" <hubicka@ucw.cz>
Subject: Re: [PATCH] Append PWD to path when using -fprofile-generate=/some/path.
Date: Wed, 20 Dec 2017 17:45:00 -0000	[thread overview]
Message-ID: <20171220174525.GK2353@tucnak> (raw)
In-Reply-To: <08c9b11b-6420-f056-ae27-38394662deca@gmail.com>

On Wed, Dec 20, 2017 at 10:35:44AM -0700, Martin Sebor wrote:
> On 10/27/2017 07:17 AM, Martin Liška wrote:
> > Hello.
> > 
> > It's improvement that I consider still useful even though we're not
> > going to use
> > it for profiled bootstrap.
> > 
> > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> For what it's worth, although it looks correct as is, I think
> the code in the patch would be clearer and less prone to off
> by-1 errors, while at the same time equally as efficient, if
> it were written in terms of strcpy and strcat:
> 
>   char *b = XNEWVEC (char,
>                      2 + strlen (pwd)
>                      + strlen (profile_data_prefix));
> 
>   strcpy (b, profile_data_prefix);
>   strcat (b, "/");
>   strcat (b, pwd);

I disagree.  One thing is that GCC attempts to optimize even bad code;
in many cases just can't as soon as there is some call in between etc.,
the other is that we just shouldn't give bad examples.
When you know the sizes, we shouldn't be lazy and rely on compiler to figure
out what we should have written.  Though on this case we have a nice
function for all of that,
  char *b = concat (profile_data_prefix, "/", pwd, NULL);

Another thing is that the "/" in there is wrong, so
  const char dir_separator_str[] = { DIR_SEPARATOR, '\0' };
  char *b = concat (profile_data_prefix, dir_separator_str, pwd, NULL);
needs to be used instead.
Does profile_data_prefix have any dir separators stripped from the end?
Is pwd guaranteed to be relative in this case?

	Jakub

  reply	other threads:[~2017-12-20 17:45 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-25 10:00 [PATCH] Introduce 4-stages profiledbootstrap to get a better profile Martin Liška
2017-05-25 10:00 ` Martin Liška
2017-05-25 11:38 ` Markus Trippelsdorf
2017-05-25 16:28   ` Martin Liška
2017-05-29 15:11     ` Jan Hubicka
2017-05-29  5:14 ` Markus Trippelsdorf
2017-06-06 13:30   ` Martin Liška
2017-06-19 10:37     ` Jan Hubicka
2017-08-30 10:44       ` [RFC] Make 4-stage PGO bootstrap really working Martin Liška
2017-09-14 12:21         ` Martin Liška
2017-10-19 12:59           ` Martin Liška
2017-10-19 14:53             ` Markus Trippelsdorf
2017-10-25 12:43         ` Markus Trippelsdorf
2017-10-27 13:07           ` Martin Liška
2017-10-27 15:00             ` Markus Trippelsdorf
2017-10-30 11:08               ` Richard Biener
2017-10-27 13:17           ` [PATCH][OBVIOUS] Fix profiledbootstrap Martin Liška
2017-10-27 13:19 ` [PATCH] Append PWD to path when using -fprofile-generate=/some/path Martin Liška
2017-12-20 14:55   ` Martin Liška
2017-12-20 17:35   ` Martin Sebor
2017-12-20 17:45     ` Jakub Jelinek [this message]
2017-12-20 18:00       ` Martin Sebor
2017-12-21  9:13       ` Martin Liška
2017-12-21 16:30         ` Martin Sebor
2018-05-16 12:26         ` [PATCH] When using -fprofile-generate=/some/path mangle absolute path of file (PR lto/85759) Martin Liška
2018-06-20 11:54           ` Martin Liška
2018-06-22 20:35           ` Jeff Law
2018-06-29 14:38             ` Martin Liška
2018-07-02  8:51               ` Rainer Orth
2018-07-20  4:02               ` Bin.Cheng
2018-07-20  8:43                 ` Martin Liška
2018-07-03  9:39           ` Jonathan Wakely

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=20171220174525.GK2353@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=mliska@suse.cz \
    --cc=msebor@gmail.com \
    --cc=richard.guenther@gmail.com \
    /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).