New issue
Advanced search Search tips

Issue 859347 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Windows PGO build: clang_rt.profile libs missing from downloaded clang install

Project Member Reported by yn...@vivaldi.com, Jul 1

Issue description

Chrome Version: v67, v68
OS: Windows 10

What steps will reproduce the problem?
(1) Build chromium target chrome with PGO and clang (GN flags chrome_pgo_phase=1 is_clang=true is_official_build=true is_component_build=false is_debug=false)

What is the expected result?

Successfully complete building

What happens instead?

First: Path to clang_rt.profile*.lib in chromium/build/config/compiler/pgo/BUILD.gn is wrong; the path needs to be wrapped in a rebase_path. This was independently reported in Chromium-dev in January https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/2767533e-8204-460d-930a-689a2c1d84b8%40chromium.org

Second: Once the library path has been fixed, linking fails because the various clang_rt.profile libs are not included in the downloaded clang (tools/clang/scripts/update.py)

I am also seeing other link errors ("___profd_*" symbols already defined), but at this time I do not know if that is caused by a local error in my branch, or is due to the missing libraries.

BTW, please note that tools/mb/mb_config.pyl still specify is_clang=false for the PGO modes. This configuration will apparently no longer build successfully, since there are changes in the source that is not compatible with MSVC, at least involving the storage class of features::kEnableDisplayZoomSetting in ui/display/display_switches.cc and ui/display/display_switches.h
 
chrome_pgo_phase and is_clang don't go together. We should probably have an assert against it. We've never tested it.

(It'll probably work some day, but that day is not today.)
That may be, but https://codereview.chromium.org/2507333002 suggests that it should (or did at one time) work.

Alternatively, if the clang PGO variant currently does not work, and assuming PGO is still supported (is it?), shouldn't the build still work? If so, then I think the Chromium source still needs to be MSVC compatible until clang PGO works.
That CL points to  bug 666152 , which isn't marked Fixed as you can see. It's a necessary but not sufficient part of making it work.

PGO only worked in MSVC, and the MSVC build hasn't been officially supported for a while. It's community-supported though, meaning we'll gladly accept patches for MSVC compat if they aren't too horrible.
(re "still needs to be": clang-built chrome/win without PGO has comparable perf to msvc-built chrome/win _with_ PGO, so I don't think that sentence holds. Also see http://blog.llvm.org/2018/03/clang-is-now-used-to-build-chrome-for.html)
OK.

It might still be an idea for you to add a GN assert, or something similar, to inform those who do try to use clang+PGO.

In any case we are still testing PGO, so changing to order files might work (I haven't investigated yet, but I assume that even with the default order files in our current official builds, we would still need to consider customized order files for our configuration)
I'll add an assert.

https://chromium.googlesource.com/chromium/src/+/master/docs/win_order_files.md might be helpful for you when you try and generate order files. I'm not even sure you need custom ones (measure to be sure, but we only update order files every now and then and that seems to be fine).

Sign in to add a comment