Add IR-based PGO #2474

JohanEngelen · 2017-12-28T13:13:36Z

First, some refactoring such that the code doesn't assume just one kind of PGO.

dnadlinger · 2017-12-28T13:50:59Z

driver/cl_options_instrumentation.h

+  PGO_IRBasedUse,
+};
+extern PGOKind pgoMode;
+inline bool isDoingPGOInstr() {


I don't have a strong opinion on this, but these names reads a bit odd – maybe isPGOInstrumenting, instrumentingForPGO, or something like that?

and then the other isUsingPGOProfile ?

dnadlinger · 2017-12-28T13:52:26Z

You have probably thought about this, but maybe an alternative naming scheme would be "AST" instead of "frontend"?

JohanEngelen · 2017-12-28T13:59:43Z

I used "frontend" because I think that's what is usually used in LLVM circles. In LLVM code, it's called "ClangPGO" (frontend based) and "LLVMPGO" (IR based).

JohanEngelen · 2017-12-28T16:16:52Z

I'm renaming it to "AST Based", I like that better indeed.
Got IR-based to work aswell, very easy :)
(and it seems that using a sample-based PGO profile is also very easy now!)

JohanEngelen · 2017-12-28T16:47:28Z

@jondegenhardt Done! :-) Important difference between AST- (old) and IR-based PGO in practical use is that the other commandline flags must be the same with IR PGO. This means that you have to specify -O3 when instrumenting with IR PGO as well. (you'll see "hash mismatch" warnings without it during -fprofile-use, meaning that the profile cannot be used and you won't get the optimizations you want)

jondegenhardt · 2017-12-28T17:12:22Z

Very nice! I won't have a chance to try it until after new year, but I'll give it shot then.

JohanEngelen · 2017-12-28T23:54:24Z

Strange, anybody know why the file is not created? The CI systems error with:

773: $ "/Users/travis/build/ldc-developers/ldc/bin/ldc2" "-O3" "-fprofile-generate=/Users/travis/build/ldc-developers/ldc/tests/PGO/Output/irbased_indirect_calls.d.tmp.profraw" "-run" "/Users/travis/build/ldc-developers/ldc/tests/PGO/irbased_indirect_calls.d"

773: $ "/Users/travis/build/ldc-developers/ldc/bin/ldc-profdata" "merge" "/Users/travis/build/ldc-developers/ldc/tests/PGO/Output/irbased_indirect_calls.d.tmp.profraw" "-o" "/Users/travis/build/ldc-developers/ldc/tests/PGO/Output/irbased_indirect_calls.d.tmp.profdata"

773: # command stderr:

773: error: /Users/travis/build/ldc-developers/ldc/tests/PGO/Output/irbased_indirect_calls.d.tmp.profraw: No such file or directory

But the lit-test works fine on my machine.

Edit: nevermind, I found the problem.

JohanEngelen · 2017-12-29T13:57:35Z

The failure on Windows is due to some bug in LLVM. I've a testcase and will submit it to their bugtracker.
So for now, I'll have to disable this on Windows...

JohanEngelen · 2017-12-29T15:17:06Z

Shall I fully disable it on Windows, or only mark the test as XFail ? (only generating the profile for a Windows target is broken; profile usage still works)
I can also give a nice error message when a Windows target is chosen for profile generation? (right now, LLVM just outputs "Error:")

JohanEngelen · 2017-12-29T15:17:38Z

(needless to say, I'd like to get this in quick, so other folks can start testing)

kinke · 2017-12-31T11:20:40Z

Thanks for doing this. Will it eventually supersede the AST-based one? [I would be looking forward to the resulting code cleanup.]

Wrt. Windows, XFAIL sounds good to me, as does a proper error message for the time being.

JohanEngelen · 2017-12-31T13:01:54Z

Will it eventually supersede the AST-based one?

Maybe. The AST-based code makes a few PGO items easier and facilitates prototyping things (like the virtual call promotion I worked on). But probably most importantly, the AST-based instrumentation is the same as what's needed for code coverage. (https://clang.llvm.org/docs/SourceBasedCodeCoverage.html)
I don't have plans right now on taking the AST-based PGO further though, so I think people should move towards the IR-based PGO (Clang's AST-based PGO also stopped progressing, and all PGO effort is on IR-based PGO afaict).

[I would be looking forward to the resulting code cleanup.]

"cleanup" :'(

I'll add the error for Windows targets. I've already sent a question about the error to LLVM folks, I think it's just a bug somewhere (I can get it to (appear to) work with llvm's cmdline tools).

kinke · 2017-12-31T13:12:26Z

"cleanup" :'(

What I actually meant was code reduction, the less code to maintain, the better ;) - even more so if scattered around the code base like this one.

JohanEngelen · 2018-01-03T19:14:22Z

driver/main.cpp

@@ -1033,6 +1032,8 @@ int cppmain(int argc, char **argv) {
    global.lib_ext = "a";
  }

+  opts::initializeInstrumentationOptionsFromCmdline();


i had to place the call here, after the triple has been initialized. The option sanity-checking needs to know the target triple, and I didn't want to separate the sanity checking from the initialization code.

JohanEngelen · 2018-01-14T17:47:58Z

Any concerns before I pull this?

dnadlinger reviewed Dec 28, 2017

View reviewed changes

JohanEngelen force-pushed the irpgo branch from edba7f5 to 7b80a26 Compare December 28, 2017 16:44

JohanEngelen changed the title ~~[WIP] Add IR-based PGO~~ Add IR-based PGO Dec 28, 2017

JohanEngelen force-pushed the irpgo branch from 7b80a26 to c60bf81 Compare December 28, 2017 19:44

JohanEngelen force-pushed the irpgo branch 2 times, most recently from 2dc9902 to 1474769 Compare December 29, 2017 00:21

JohanEngelen force-pushed the irpgo branch from 1474769 to 78be232 Compare December 31, 2017 14:40

JohanEngelen commented Jan 3, 2018

View reviewed changes

JohanEngelen added 2 commits January 15, 2018 23:59

PGO: refactor such that there can be IR-based and frontend-based PGO.

3ec7917

PGO: Add IR-based PGO.

e8e28b3

JohanEngelen force-pushed the irpgo branch from 78be232 to e8e28b3 Compare January 15, 2018 23:03

JohanEngelen merged commit 7f087f4 into ldc-developers:master Jan 18, 2018

JohanEngelen deleted the irpgo branch January 18, 2018 21:19

JohanEngelen mentioned this pull request Jan 26, 2018

IR PGO not working on Windows #2539

Closed

andjo403 mentioned this pull request Mar 13, 2018

Add basic PGO support. rust-lang/rust#48346

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IR-based PGO #2474

Add IR-based PGO #2474

JohanEngelen commented Dec 28, 2017

dnadlinger Dec 28, 2017 •

edited

Loading

JohanEngelen Dec 28, 2017

dnadlinger commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

jondegenhardt commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017 •

edited

Loading

JohanEngelen commented Dec 29, 2017

JohanEngelen commented Dec 29, 2017

JohanEngelen commented Dec 29, 2017

kinke commented Dec 31, 2017

JohanEngelen commented Dec 31, 2017

kinke commented Dec 31, 2017

JohanEngelen Jan 3, 2018

JohanEngelen commented Jan 14, 2018

Add IR-based PGO #2474

Add IR-based PGO #2474

Conversation

JohanEngelen commented Dec 28, 2017

dnadlinger Dec 28, 2017 • edited Loading

Choose a reason for hiding this comment

JohanEngelen Dec 28, 2017

Choose a reason for hiding this comment

dnadlinger commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017

jondegenhardt commented Dec 28, 2017

JohanEngelen commented Dec 28, 2017 • edited Loading

JohanEngelen commented Dec 29, 2017

JohanEngelen commented Dec 29, 2017

JohanEngelen commented Dec 29, 2017

kinke commented Dec 31, 2017

JohanEngelen commented Dec 31, 2017

kinke commented Dec 31, 2017

JohanEngelen Jan 3, 2018

Choose a reason for hiding this comment

JohanEngelen commented Jan 14, 2018

dnadlinger Dec 28, 2017 •

edited

Loading

JohanEngelen commented Dec 28, 2017 •

edited

Loading