Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FDO builds use -X{gcc/clang}-only #1171

Closed
s-kanev opened this issue Apr 21, 2016 · 5 comments
Closed

FDO builds use -X{gcc/clang}-only #1171

s-kanev opened this issue Apr 21, 2016 · 5 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Milestone

Comments

@s-kanev
Copy link
Contributor

s-kanev commented Apr 21, 2016

IIRC, these are supported by the internal crosstool, but my upstream gcc 5.2.1 and clang 3.6.2 don't like either one:

gcc: error: unrecognized command line option ‘-Xgcc-only=-fprofile-generate=/path’
gcc: error: unrecognized command line option ‘-Xclang-only=-fprofile-instr-generate=/path’
clang: warning: argument unused during compilation: '-Xclang-only=-fprofile-instr-generate=/path'
clang: warning: argument unused during compilation: '-Xgcc-only=-fprofile-generate=/path'

This is obviously low-priority because folks who care enough about setting up an FDO build process can customize the toolchain to include compiler-specific flags. But it would be nice if --fdo_{instrument/optimize} "just work" externally.

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java#L721

@damienmg damienmg added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) category: native rules (Java / C++ / Python) labels Apr 22, 2016
@ulfjack
Copy link
Contributor

ulfjack commented May 4, 2016

We need to update our CROSSTOOL file and add features for fdo_instrument and fdo_optimize; that'll make the error go away.

You can do that in your own client, or send a patch - just copy the corresponding sections from:
third_party/bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
to:
tools/cpp/CROSSTOOL.tpl (I think, maybe also to tools/cpp/CROSSTOOL)

and then update the flags to remove the -Xgcc-only prefixes. Even better to add code to cc_configure to auto-detect gcc / clang and add the correct flags.

s-kanev added a commit to s-kanev/XIOSim that referenced this issue May 8, 2016
Copied from the bazel defaults for GCC, which seem to include some
internal crosstool flags (bazelbuild/bazel#1171).

FDO gives us ~15% speedup. Probably worth switching the CI server to use it.

Change-Id: I655eb29b865f51a2e51838c82f28f78db0cbc1b7
@meteorcloudy
Copy link
Member

I've moved these features from CppConfiguration.java to CROSSTOOL.tpl and using cc_configure to add corresponding flags, FDO build works fine on Linux with gcc. But on OS X, building with clang, I still get this error:
clang: error: unknown argument: '-fprofile-instr-generate=/Users/pcloudy/fdo_output'

It turns out the fdo options of clang actually work like this:
http://clang.llvm.org/docs/UsersManual.html#profiling-with-instrumentation.

Clang doesn't accept -fprofile-instr-generate=, but accepts -fprofile-generate= and -fprofile-use=, which is compatible with gcc. But only in version 3.7 or higher.

I guess I'll just use the same flag for both gcc and clang, and when people get errors with clang, they have to update it. Is it the right thing to do?

@ulfjack
Copy link
Contributor

ulfjack commented May 24, 2016

Yes. It would be even better if we could detect the clang version, but I'm not sure it's worth the effort. Maybe we can have an attribute on cc_configure to allow people to override the behavior as a workaround?

@benjaminp
Copy link
Collaborator

I posted https://bazel-review.googlesource.com/c/bazel/+/17470 to make fdo work out of the box for gcc and clang.

@meteorcloudy
Copy link
Member

meteorcloudy commented Sep 29, 2017

@benjaminp Great! Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants