[PATCH] D35807: Add test coverage for new PM PGOOpt handling.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Awesome, I'm glad to see a nice way of testing this at a low level!



================
Comment at: lib/Passes/PassBuilder.cpp:175-178
+static cl::opt<std::string> PGOOptString(
+    "pgo-opt", cl::init(""), cl::Hidden,
+    cl::desc("PGOOpt string in the format of "
+             "{gen|use|sample_gen|sample_use}=value"));
----------------
Rather than these options going in PassBuilder.cpp and overriding things, I think this should go into tools/opt/NewPMDriver.cpp much like the ThinLTO controls do.

Then the opt tool can construct a PGOOpt struct and hand it into the PassBuilder much the same way Clang will.

Also, `sample_gen` doesn't appear to be valid?

I think rather than having one flag that you have to parse I would suggest have three flags: `pgo-instr-gen`, `pgo-intsr-use`, and `pgo-sample-use`. I think there is a way in the `cl::opt` layer to say the flags are mutually incompatible, but even if you have to do that manually, it seems better than parsing an '=' out of the flag value.


https://reviews.llvm.org/D35807