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

Dehao Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 25 15:27:44 PDT 2017

danielcdh marked an inline comment as done.
danielcdh added inline comments.

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"));
chandlerc wrote:
> 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.
pgo-instr-{gen|use}, conflict with existing flags. Used pgo-{gen|use|sample-use} instead.


More information about the llvm-commits mailing list