[PATCH] D30913: [NFC] Feature generic options to setup start/stop-after/before
Quentin Colombet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 14 15:57:47 PDT 2017
qcolombet added a comment.
Hi Matthias,
Thanks for the clarifications.
I am ok moving some more error checking within the TargetMachine class, but I have one more question regarding not exposing the API for the options, see my inline comment.
Cheers,
-Quentin
================
Comment at: tools/llc/llc.cpp:479-485
+ AnalysisID StartBeforeID = TargetMachine::getStartBeforeID();
+ AnalysisID StartAfterID = TargetMachine::getStartAfterID();
+ AnalysisID StopAfterID = TargetMachine::getStopAfterID();
+ AnalysisID StopBeforeID = TargetMachine::getStopBeforeID();
+
if (!RunPassNames->empty()) {
+ if (StartAfterID || StopAfterID || StartBeforeID || StopBeforeID) {
----------------
MatzeB wrote:
> qcolombet wrote:
> > MatzeB wrote:
> > > Maybe keep it simple and move all of this into TargetMachine and just keep a TargetMachine::hasLimitedPipelineOption() or so to check for the start/stop with run-pass error?
> > You mean something like:
> > getRunPass(StringRef PassName, bool AbortIfNotRegistered, bool HasLimitedPipelineOption)
> >
> > And going farther, having the AbortXX and HasLimitedXX being field member?
> What I was getting at:
>
> - Currently you expose 7 function and an enum type as API for something that looks like a few commandline options that we should be able to handle inside of TargetMachine.cpp entirely.
> - However when you move everything into TargetMachine.cpp we obviously cannot do the
> `if (StartAfterID || StopAfterID || StartBeforeID || StopBeforeID) {` here anymore. That's why I suggested to expose a single function in the API which looks similar to this:
> `class TargetMachine { ... bool hasLimitedPipeline() { return StartAfterID || StopAfterID || StartBeforeID || StopBeforeID; } ... }`
> you could use that here and at that point do not need the {Start|Stop}XXXID variables and all the API to access them anymore.
Ok, but how do we set the default arguments for this API then?
```
TargetMachine ::addPassesToEmitFile
```
Should we just get rid of those arguments and all do that internally?
(Note: I am not a fan of changing the pipeline behind the developer back.)
Repository:
rL LLVM
https://reviews.llvm.org/D30913
More information about the llvm-commits
mailing list