Add the possibility to build with PGO when using Clang

Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152
Committed: https://crrev.com/0454d1c39637beb1fb084bea057d614f6908a7ae
Cr-Commit-Position: refs/heads/master@{#436067}

[inglorion, 2016-11-17 00:31:03]

Description was changed from

==========
added the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=
==========

to

==========
added the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=
==========

[inglorion, 2016-11-17 00:31:03]

inglorion@chromium.org changed reviewers:
+ hans@chromium.org, sebmarchand@chromium.org

[inglorion, 2016-11-17 00:33:11]

Please take a look. I imagine this needs some polishing.

[hans, 2016-11-17 00:46:08]

hans@chromium.org changed reviewers:
+ thakis@chromium.org

[hans, 2016-11-17 00:46:08]

Nice!

This goes beyond my gn-foo, so I think Sebastien and Nico can comment better on
the actual implementation.


From what I understand, this introduces a new PGO config, but it's only used
with Clang. What's the relationship to how the existing MSVC-based PGO build is
configured?



Some very minor remarks:

> added the possibility to build with PGO when using Clang

It's more common for Chromium commits to use the imperitive form ("Add the
possibility..") and proper capitalization. (Some even end the first line with a
period, but I usually don't :-)

>
> r257133 introduced chrome_pgo_phase to allow building Chrome with
> profile-guided optimization. Thus far, support for this has only
> been implemented for MSVC on Windows. This change allows PGO to
> be used with Clang, as well.
>
> BUG=

I suspect there might be follow-ups, so it would be good to file a "Make PGO
work with Clang" tracker bug, mark it as blocking crbug.com/82385, and reference
it here.

[inglorion, 2016-11-17 01:32:14]

Description was changed from

==========
added the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=
==========

to

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=663318
==========

[inglorion, 2016-11-17 01:35:49]

Description was changed from

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=663318
==========

to

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=
==========

[inglorion, 2016-11-17 01:40:54]

Description was changed from

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=
==========

to

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152
==========

[inglorion, 2016-11-17 01:53:39]

Thanks for your comments, hans! I've changed the description to imperative
style, created a bug, linked it from here, and marked it as blocking
crbug.com/82385.

As for your question:

> From what I understand, this introduces a new PGO config, but it's only used
> with Clang. What's the relationship to how the existing MSVC-based PGO build
is
> configured?

This reuses the same chrome_pgo_phase to control the flags we pass: 0 means no
PGO, 1 means add the instrumentation for collecting profiling data, and 2 means
use profiling data to optimize the code.

This CL also adds an additional variable, pgo_data_path, which is used to
specify the path to read profiling data from when chrome_pgo_phase == 2.

I've used this on Linux to create a build of Chromium with PGO. On Windows, I
still have a little bit of work to do to make it all work. I have a patch that
will make -fprofile-generate work with clang-cl, which allows me to build
executables with PGO, but the build files we have call lld-link instead of going
through clang-cl, and that doesn't currently do the right thing.

Once we have this working on Linux and Windows, I imagine we will want to take a
look at how to factor it - maybe the PGO bits should live in their own
directory, as I've done here, or maybe we want to move it all to the compiler or
toolchain directories.

[Sébastien Marchand, 2016-11-17 17:31:15]

I like the idea of moving the PGO settings into their own files, but I think
that the pgo subdirectory should live in build/config/compiler. So far it look
like it's introducing some confusion because the MSVC and the Clang bits are
living in a different place, but I'm ok with this (as long as you add a TODO and
open a crbug asking me to move the config into these new files)

I've expressed my concern about the fact that this invert the current approach
of how we enable PGO (currently we need to explicitly enable it on a per target
basis, but your CL propose the opposite, enabled for all the targets except if
we disable it, like you do for NaCl).

https://codereview.chromium.org/2507333002/diff/1/build/config/BUILDCONFIG.gn
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/2507333002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:481: "//build/config/pgo:default_pgo_flags",
My concern with this is that it seems to be a global way to enable PGO on *all*
the targets? We really want to be able to turn on PGO only for some targets (on
Windows at least), so I'm all in for moving the PGO specific code into a build
config (instead of having it living in chrome/BUILD.gn), but I don't think that
it should be a part of the default config?

https://codereview.chromium.org/2507333002/diff/1/build/config/pgo/BUILD.gn
File build/config/pgo/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/1/build/config/pgo/BUILD.gn#n...
build/config/pgo/BUILD.gn:24: # Flags are only added if PGO is enabled, so that
this config is safe to
I'm not sure that this is safe to assume that this config will stay empty if PGO
is not enabled, it'll likely grow (e.g. I'll move the Windows code into it, and
maybe one day we'll look at using PGO on OS X / Android), and it's not unlikely
that someone will overlook this comment and add some default compiler/linker
flags... Or link to another config that turn on some flags by default.

https://codereview.chromium.org/2507333002/diff/1/build/config/pgo/BUILD.gn#n...
build/config/pgo/BUILD.gn:30: if (is_clang && !is_nacl) {
Could you add some TODO (assigned to me) to move the MSVC code here?

https://codereview.chromium.org/2507333002/diff/1/chrome/test/data/nacl/BUILD.gn
File chrome/test/data/nacl/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/1/chrome/test/data/nacl/BUILD...
chrome/test/data/nacl/BUILD.gn:606: configs -= [
"//build/config/pgo:default_pgo_flags" ]
Again, I'll do the opposite, include it when you need it.

[inglorion, 2016-11-18 02:07:36]

Hi Sébastien,

Thank you for your review. Addressing your points:

On 2016/11/17 17:31:15, Sébastien Marchand wrote:
> I like the idea of moving the PGO settings into their own files, but I think
> that the pgo subdirectory should live in build/config/compiler.

Cool, I'll move it there. Were you thinking of build/config/compiler/pgo or just
build/config/compiler?

> So far it look
> like it's introducing some confusion because the MSVC and the Clang bits are
> living in a different place, but I'm ok with this (as long as you add a TODO
and
> open a crbug asking me to move the config into these new files)

Yeah, I was thinking we would want to make a pass to get them both on the same
code path.

> I've expressed my concern about the fact that this invert the current approach
> of how we enable PGO (currently we need to explicitly enable it on a per
target
> basis, but your CL propose the opposite, enabled for all the targets except if
> we disable it, like you do for NaCl).

I understand your concern here. Let me first explain why I did it this way. I
followed the way it's done for sanitizers, which solves a specific problem: Both
the instrumentation for PGO and sanitizers puts code in the object files that
requires them to be linked with some library. That library, in turn, requires
libc to be linked in. However, we link some parts of the code with -nostdlib,
which explicitly disables linking in libc. This would cause undefined
references. To avoid that problem, we put all the PGO flags in a configuration,
and we disable that configuration for the parts that we link with -nostdlib.
This gets rid of all the PGO flags for those targets, and they link correctly.
Essentially, removing :default_pgo_flags is like saying "Don't use PGO, even if
we're doing a PGO build." I feel that *should* be the exception rather than the
norm, which is why the configuration is enabled for all targets, except the ones
that explicitly opt out. Again, this follows how it's done for sanitizers.

As for your concern that we may one day inadvertently add flags to
:default_pgo_flags - would it work if we did something like defining
:pgo_generate and :pgo_use configs that unconditionally set the flags to
generate and use profile data, and have :default_pgo_flags include the right
config based on the value of chrome_pgo_phase?

[Sébastien Marchand, 2016-11-18 16:55:25]

On 2016/11/18 02:07:36, inglorion wrote:
> Hi Sébastien,
> 
> Thank you for your review. Addressing your points:
> 
> On 2016/11/17 17:31:15, Sébastien Marchand wrote:
> > I like the idea of moving the PGO settings into their own files, but I think
> > that the pgo subdirectory should live in build/config/compiler.
> 
> Cool, I'll move it there. Were you thinking of build/config/compiler/pgo or
just
> build/config/compiler?

I'll go with build/config/compiler/pgo so it's more isolated.

> 
> > So far it look
> > like it's introducing some confusion because the MSVC and the Clang bits are
> > living in a different place, but I'm ok with this (as long as you add a TODO
> and
> > open a crbug asking me to move the config into these new files)
> 
> Yeah, I was thinking we would want to make a pass to get them both on the same
> code path.

Agree, and as I said I think that it's safe to do this in 2 CLs.

> 
> > I've expressed my concern about the fact that this invert the current
approach
> > of how we enable PGO (currently we need to explicitly enable it on a per
> target
> > basis, but your CL propose the opposite, enabled for all the targets except
if
> > we disable it, like you do for NaCl).
> 
> I understand your concern here. Let me first explain why I did it this way. I
> followed the way it's done for sanitizers, which solves a specific problem:
Both
> the instrumentation for PGO and sanitizers puts code in the object files that
> requires them to be linked with some library. That library, in turn, requires
> libc to be linked in. However, we link some parts of the code with -nostdlib,
> which explicitly disables linking in libc. This would cause undefined
> references. To avoid that problem, we put all the PGO flags in a
configuration,
> and we disable that configuration for the parts that we link with -nostdlib.
> This gets rid of all the PGO flags for those targets, and they link correctly.
> Essentially, removing :default_pgo_flags is like saying "Don't use PGO, even
if
> we're doing a PGO build." I feel that *should* be the exception rather than
the
> norm, which is why the configuration is enabled for all targets, except the
ones
> that explicitly opt out. Again, this follows how it's done for sanitizers.

I think that the PGO config should not mirror what we're doing with the
sanitizers.
The builds produced with the sanitizer config are mostly for a lab environment
(or for
a really small user population) and so it's fine to use a blacklist here (i.e.
explicitly disable the sanitizer config for some targets). The PGO builds on the
other
hand are aimed to be shipped to a really large population, so we need to be more
careful with this (PGO might have a significant impact on differential patch
size).
E.g. on Windows I had to restrict this tochrome_child.dll because of the
increase on
build time and on patch size.

I think that what you really want to do is to apply LTCG (or WPO depending on
how you
call it) everywhere and link some specific targets with PGO. This is what we do
for the
MSVC build, see how the full_wpo_on_official flag is uses.


> 
> As for your concern that we may one day inadvertently add flags to
> :default_pgo_flags - would it work if we did something like defining
> :pgo_generate and :pgo_use configs that unconditionally set the flags to
> generate and use profile data, and have :default_pgo_flags include the right
> config based on the value of chrome_pgo_phase?

Yeah, this should be fine.

[inglorion, 2016-11-24 00:53:21]

Thanks again for your review, Sébastien. I have made the following changes:

* PGO configuration files moved to build/config/compiler/pgo as per your
request.

* I also moved the MSVC configuration for PGO into the same location, as I
needed to touch it anyway.

I tested that this still works with MSVC, and was able to do an instrumented
build (chrome_pgo_phase == 1), gather profiling data, and build an optimized
build (chrome_pgo_phase == 2). I did have to manually run pgosweep - I don't
know if that's expected or not.

[Sébastien Marchand, 2016-11-24 15:57:07]

Thanks for doing this! I've a few extra comments but I like this approach.

This is expected that you need to run pgosweep manually during the profiling
step. I've added this script in Chrome to automate the profiling step:
https://cs.chromium.org/chromium/src/build/win/run_pgo_profiling_benchmarks.py ,
I think that it'd be cool to make it work with Clang-PGO as well (in a separate
CL). By default this script use Telemetry to run a bunch of benchmark with the
'win_pgo_profiler' config, see
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry...
to understand how this profiler config works in Telemetry.

I think that we should sync on what's next, as our work will probably overlap.
I'll be in MTV in 2 weeks (week of Dec 5th), and I'll try to spend some time
with your team.

https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
File build/config/compiler/pgo/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
build/config/compiler/pgo/BUILD.gn:12: config("default_pgo_ldflags") {
If we stick with this approach I'd prefer to keep this config as small as
possible, e.g:

config("default_pgo_flags") {
  config += ["pgo_common_flags"]
  if (chrome_pgo_phase == 1) {
    config += ["pgo_instrumentation_flags"]
  } else if (chrome_pgo_phase == 2) {
    config += ["pgo_optimization_flags"]
  } 
}

You can even split the pgo_instrumentation_flags/pgo_optimization_flags into
separate config (e.g. pgo_clang_optimization_flags,
pgo_msvc_instrumentation_flags...)

Please keep the linker and the compiler flags in the same config, ld is really
Clang specific so it's weird to have it in the name of a MSVC config.

https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
build/config/compiler/pgo/BUILD.gn:81: # It's possible to have some profile data
legitimately missing, and at least some
> 80 characters.

https://codereview.chromium.org/2507333002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/20001/chrome/BUILD.gn#newcode127
chrome/BUILD.gn:127: configs += [
"//build/config/compiler/pgo:default_pgo_flags" ]
This config will be applied to chrome.exe, do you also want to apply it to the
'main_dll' target (for the Clang build only).

Again, I really think that this should be behind a "if (chrome_pgo_phase > 0)"
condition. Debugging an error caused by someone adding some default flag to the
PGO config will probably be tricky, so it's probably better to add some more
safety checks.

[inglorion, 2016-12-02 02:10:49]

On 2016/11/24 15:57:07, Sébastien Marchand wrote:
>
https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
> build/config/compiler/pgo/BUILD.gn:12: config("default_pgo_ldflags") {
> If we stick with this approach I'd prefer to keep this config as small as
> possible, e.g:

Done. default_pgo_flags is now a simple switch based on chrome_pgo_phase, with a
comment in the == 0 case to explain that it is important that no flags be
defined there.

> You can even split the pgo_instrumentation_flags/pgo_optimization_flags into
> separate config (e.g. pgo_clang_optimization_flags,
> pgo_msvc_instrumentation_flags...)

I tried, but this runs into problems: gn tries to expand variables in strings in
configs, but not all variables are defined in all builds, so it will fail, for
example, to expand $clang_base_path when building with MSVC. So I ended up
splitting the configs by what they do (instrumentation, optimization), but
selecting the right flags for the compiler using an if statement.

> Please keep the linker and the compiler flags in the same config, ld is really
> Clang specific so it's weird to have it in the name of a MSVC config.

Done.

>
https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
> build/config/compiler/pgo/BUILD.gn:81: # It's possible to have some profile
data
> legitimately missing, and at least some
> > 80 characters.

Fixed.

>
https://codereview.chromium.org/2507333002/diff/20001/chrome/BUILD.gn#newcode127
> chrome/BUILD.gn:127: configs += [
> "//build/config/compiler/pgo:default_pgo_flags" ]
> This config will be applied to chrome.exe, do you also want to apply it to the
> 'main_dll' target (for the Clang build only).

I'm planning to take a look at which targets I want to apply PGO to after this
patch is committed, if that's ok.

> Again, I really think that this should be behind a "if (chrome_pgo_phase > 0)"
> condition. Debugging an error caused by someone adding some default flag to
the
> PGO config will probably be tricky, so it's probably better to add some more
> safety checks.

Does my new default_pgo_flags sufficiently take care of this, or do you still
want me to go in and add a check for chrome_pgo_phase everywhere that config is
used? I prefer to go without that, but I'll add extra checks if you really think
it's better.

Thanks again for your comments!

[Nico, 2016-12-02 02:48:11]

I only glanced through, looks great to me. lgtm once Seb/Hans are happy.

https://codereview.chromium.org/2507333002/diff/40001/build/config/compiler/p...
File build/config/compiler/pgo/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/40001/build/config/compiler/p...
build/config/compiler/pgo/BUILD.gn:69: "-Wno-error=profile-instr-out-of-date",
A general remark: We want chrome builds to be silent. If something goes wrong,
it's either a problem, in which case it should be an error, or it isn't, in
which case it shouldn't print anything. So I think this means this should be
-Wno-profile-instr-out-of-date instead – but if this is considered normal, then
the warning is useless upstream as-is. So we should then tweak the warning
upstream to be useful, or remove it upstream.

[Sébastien Marchand, 2016-12-02 13:24:48]

Thanks for addressing all my comments! This approach lgtm, thanks for
taking care of the MSVC part!

One last comment :), when replying to the comments in a CL could you try
to do it via the crbug UI (instead of replying to the email). This way
the new comments will be next to the code and it's easier to make sure
that they've all been addressed. See
https://codereview.chromium.org/2527533003/diff/80001/syzygy/agent/asan/heap_...
as an example.

On 2016/12/02 02:10:49, inglorion wrote:
> On 2016/11/24 15:57:07, Sébastien Marchand wrote:
> >
>
https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
> > build/config/compiler/pgo/BUILD.gn:12: config("default_pgo_ldflags") {
> > If we stick with this approach I'd prefer to keep this config as small as
> > possible, e.g:
> 
> Done. default_pgo_flags is now a simple switch based on chrome_pgo_phase, with
a
> comment in the == 0 case to explain that it is important that no flags be
> defined there.

Awesome! 

> 
> > You can even split the pgo_instrumentation_flags/pgo_optimization_flags into
> > separate config (e.g. pgo_clang_optimization_flags,
> > pgo_msvc_instrumentation_flags...)
> 
> I tried, but this runs into problems: gn tries to expand variables in strings
in
> configs, but not all variables are defined in all builds, so it will fail, for
> example, to expand $clang_base_path when building with MSVC. So I ended up
> splitting the configs by what they do (instrumentation, optimization), but
> selecting the right flags for the compiler using an if statement.

Ok, thanks for trying that!

> 
> > Please keep the linker and the compiler flags in the same config, ld is
really
> > Clang specific so it's weird to have it in the name of a MSVC config.
> 
> Done.
> 
> >
>
https://codereview.chromium.org/2507333002/diff/20001/build/config/compiler/p...
> > build/config/compiler/pgo/BUILD.gn:81: # It's possible to have some profile
> data
> > legitimately missing, and at least some
> > > 80 characters.
> 
> Fixed.
> 
> >
>
https://codereview.chromium.org/2507333002/diff/20001/chrome/BUILD.gn#newcode127
> > chrome/BUILD.gn:127: configs += [
> > "//build/config/compiler/pgo:default_pgo_flags" ]
> > This config will be applied to chrome.exe, do you also want to apply it to
the
> > 'main_dll' target (for the Clang build only).
> 
> I'm planning to take a look at which targets I want to apply PGO to after this
> patch is committed, if that's ok.

That's entirely ok and I think that it's the safest thing to do (use a whitelist
approach rather than a blacklist).

> 
> > Again, I really think that this should be behind a "if (chrome_pgo_phase >
0)"
> > condition. Debugging an error caused by someone adding some default flag to
> the
> > PGO config will probably be tricky, so it's probably better to add some more
> > safety checks.
> 
> Does my new default_pgo_flags sufficiently take care of this, or do you still
> want me to go in and add a check for chrome_pgo_phase everywhere that config
is
> used? I prefer to go without that, but I'll add extra checks if you really
think
> it's better.

Nop, that's already really good as is.

> 
> Thanks again for your comments!

[hans, 2016-12-02 17:47:44]

lgtm too

[inglorion, 2016-12-02 19:55:51]

The CQ bit was checked by inglorion@chromium.org

[commit-bot: I haz the power, 2016-12-02 19:56:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 20:03:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 20:03:52]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[inglorion, 2016-12-02 21:39:11]

Silly me. Configs are always evaluated, even if they aren't used, so I can't
assert that a variable is defined when it's not always required to be defined. I
wrapped the contents of pgo_instrumentation_flags in if (chrome_pgo_phase == 1)
and the contents of pgo_optimization_flags in if (chrome_pgo_phase == 2), which
should fix the problem.

[inglorion, 2016-12-02 21:39:20]

The CQ bit was checked by inglorion@chromium.org

[inglorion, 2016-12-02 21:39:21]

The patchset sent to the CQ was uploaded after l-g-t-m from hans@chromium.org,
sebmarchand@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2507333002/#ps60001
(title: "Add the possibility to build with PGO when using Clang")

[commit-bot: I haz the power, 2016-12-02 21:40:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[inglorion, 2016-12-02 21:44:25]

https://codereview.chromium.org/2507333002/diff/40001/build/config/compiler/p...
File build/config/compiler/pgo/BUILD.gn (right):

https://codereview.chromium.org/2507333002/diff/40001/build/config/compiler/p...
build/config/compiler/pgo/BUILD.gn:69: "-Wno-error=profile-instr-out-of-date",
Sorry, I missed your comment earlier. I plan to follow up on this. Probably I'll
submit a patch to change this to -Wno- instead of -Wno-error= so that at least
we won't be bombarded with warnings. I'm not sure yet what the right long-term
solution is; I can see the case both for having the warnings and for not having
them, so maybe the right answer is to make it configurable.

[commit-bot: I haz the power, 2016-12-02 23:04:22]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480714760350780,
"parent_rev": "034be82f71ec0773f44fa438e546f7baf042c619", "commit_rev":
"c36e9ddedb9a5f9e770d986ecd38267119e044a9"}

[commit-bot: I haz the power, 2016-12-02 23:04:51]

Description was changed from

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152
==========

to

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152
==========

[commit-bot: I haz the power, 2016-12-02 23:04:52]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-02 23:07:52]

Description was changed from

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152
==========

to

==========
Add the possibility to build with PGO when using Clang

r257133 introduced chrome_pgo_phase to allow building Chrome with
profile-guided optimization. Thus far, support for this has only
been implemented for MSVC on Windows. This change allows PGO to
be used with Clang, as well.

BUG=666152

Committed: https://crrev.com/0454d1c39637beb1fb084bea057d614f6908a7ae
Cr-Commit-Position: refs/heads/master@{#436067}
==========

[commit-bot: I haz the power, 2016-12-02 23:07:53]

Patchset 4 (id:??) landed as
https://crrev.com/0454d1c39637beb1fb084bea057d614f6908a7ae
Cr-Commit-Position: refs/heads/master@{#436067}