-
-
Notifications
You must be signed in to change notification settings - Fork 265
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
Profile-guided optimizations (PGO) #1219
Conversation
db87aec
to
0eb48ba
Compare
5b2e2c9
to
e0125d8
Compare
This random microbenchmark for std.regex does not compile with import std.stdio, std.conv, std.array, std.regex, std.utf,
std.algorithm, std.exception;
string reEncode(string s) {
validate(s); // Throw if it's not a well-formed UTF string
static string rep(Captures!string m) {
auto c = canFind("0123456789#", m[1]) ? "#" ~ m[1] : m[1];
return text(m.hit.length / m[1].length) ~ c;
}
return std.regex.replace!rep(s, regex(`(.|[\n\r\f])\1*`, "g"));
}
string reDecode(string s) {
validate(s); // Throw if it's not a well-formed UTF string
static string rep(Captures!string m) {
string c = m[2];
if (c.length > 1 && c[0] == '#')
c = c[1 .. $];
return replicate(c, to!int(m[1]));
}
auto r=regex(`(\d+)(#[0123456789#]|[\n\r\f]|[^0123456789#\n\r\f]+)`
, "g");
return std.regex.replace!rep(s, r);
}
void rle() {
pragma(LDC_never_inline);
auto s = "??????????????\nWWWWWWWWWWWWBWWWWWWWWWWW" ~
"WBBBWWWWWWWWWWWWWWWWWWWWWWWWBWWWWWWWWWWWWWW\n" ~
"11#222##333";
enforce(s == reDecode(reEncode(s)));
}
void main() {
foreach (_; 0..100000) {
rle();
}
}
|
@klickverbot Thanks for the testcase. I get a different error though: assert in PGO code. Traversing DMD's AST is non-trivial :/ |
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
//===--- CodeGenPGO.cpp - PGO Instrumentation for LLVM CodeGen --*- C++ -*-===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you merge this into the preceding LDC header comment? Maybe just note that it is adapted from CodeGenPGO.cpp
, and that it is under LLVM's license. The reference to LICENSE.TXT also doesn't make a lot of sense in our repository.
b535efe
to
bd5bd90
Compare
f3b87eb
to
e53a5b9
Compare
Johan, take a look at this too at some point: http://reviews.llvm.org/D15829 |
I talk to myself often too, but not yet on github. |
Rebased onto master! |
Now also working for LLVM 3.9 |
5c43197
to
b2bef6f
Compare
@klickverbot @redstar The work is done! Please review, and merge ;-) (This PR is becoming larger and larger. By now, so much time has gone into this that i am very biased towards it. It's been tested to safely compile Phobos/druntime. The frontend changes are not there for the fun of it. Some frontend changes have been submitted upstream, but dlang/dmd#5501 was met with disappointing review, especially considering [time investment PGO] vs [apparent review time].) |
…sed as template alias parameter.
GREEN, also on AppVeyor! :-) |
Congratz! 👍 |
Let's just merge this, then. What's the worst that could happen? ;P (We can always back out the changes if needed, with only the negligible |
Would it be a good idea to put up a CircleCI job that builds everything with instrumentation enabled? |
Btw, for using IRPGO we also need compiler-rt. So if later on, IRPGO starts to rock more, we can pull out a bit of the PGO code and only keep it for coverage (LLVM coverage annotation is on my todo list, but it's too long already...). |
Maybe? Build LDC with instrumentation -> build phobos -> rebuild LDC using instrumentation? |
Really happy about this :) Now I need to write that promised blog post. |
This would be a next step, I guess. I was more worried about just getting decent test coverage for the PGO code besides sporadically trying to build wekapp with it (specifically the AST-facing side of things on more complex code). Or did I miss something along these lines? |
It's a good point. I think not all bugs I found during development made it into the testcases. What we could do is add a test job that builds Phobos's unittests with instrumentation enabled. That should give decent coverage of complex ASTs with |
Actually testing the profile-instr-use part is of course very welcome as well, but my suggestion for a first step was much more trivial – just add |
Interesting. e8e28b3 added -fprofile-generate clang supports -fprofile-generate (IR based; the interface is the same as GCC but the underlying implementation is very different) and -fprofile-instr-generate (AST-based; used by -fcoverage-mapping). IMHO -fprofile-instr-generate is rarely used outside of -fcoverage-mapping. Most PGO users use -fprofile-generate. Has anyone compared ldc's -fprofile-generate and -fprofile-instr-generate? |
@jondegenhardt Perhaps has tested the difference. |
Can't use IR based PGO with my tools, as IR based PGO doesn't work with LTO, and I combine PGO and LTO in my build scripts. The bug report: #2582 . I could setup tests of AST-based PGO vs IR-based PGO, without LTO, but that would involve it's own bias, at least in my tools. The tools and benchmark suite I have setup makes significant use of druntime/phobos in tight loops, and those facilities need LTO to make the IR available to the optimizer. |
See this page on the wiki that documents the work: http://wiki.dlang.org/LDC_LLVM_profiling_instrumentation
This PR implements PGO as Clang/LLVM does it:
ldc2 -fprofile-instr-generate test.d -of=test1
./test1
This generates a "default.profraw" file.
llvm-profdata merge default.profraw -output test.profdata
ldc2 -profile-instr-use=test.profdata test.d -of=test2
Instead of adding the LLVM profiler runtime to druntime like the PR does now, I think it should be a separate library that can be linked to in case of
-fprofile-instr-generate
.