CommandFlags.h/llc: Move StopAfter/StartBefore options to llc.

[MatzeB]

Summary

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

Move those two options to llc:

The options in CommandFlags.h are shared by dsymutil, gold, llc,
llvm-dwp, llvm-lto, llvm-mc, lto, opt.

-stop-after/-start-after only affect codegen passes however only gold and llc
actually create codegen passes and I believe these flags to be only
useful for users of llc. For the other tools they are just highly
confusing: -stop-after claims to "Stop compilation after a specific
pass" which is not true in the context of the "opt" tool.

[MatzeB]

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

I just realized that I can answer my own question: neither StartAfter nor StopAfter are used in gold-plugin.cpp so they can't have any effect in the gold plugin.

[tejohnson]

In D23050#503612, @MatzeB wrote:

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

I just realized that I can answer my own question: neither StartAfter nor StopAfter are used in gold-plugin.cpp so they can't have any effect in the gold plugin.

Sorry for the delayed response. I'm not personally familiar with -stop-after and -start-after. However, the gold-plugin will pass along any unrecognized options to the LLVM backend. So presumably a gold plugin user could specify them like -Wl,-plugin-opt,-start-after=pass-name.

I assume they are useful for debugging? Are they currently also supported for clang? E.g. clang -O2 -mllvm -start-after=pass-name ...?

[MatzeB]

In D23050#503625, @tejohnson wrote:

In D23050#503612, @MatzeB wrote:

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

I just realized that I can answer my own question: neither StartAfter nor StopAfter are used in gold-plugin.cpp so they can't have any effect in the gold plugin.

Sorry for the delayed response. I'm not personally familiar with -stop-after and -start-after. However, the gold-plugin will pass along any unrecognized options to the LLVM backend. So presumably a gold plugin user could specify them like -Wl,-plugin-opt,-start-after=pass-name.

I assume they are useful for debugging? Are they currently also supported for clang? E.g. clang -O2 -mllvm -start-after=pass-name ...?

Yes they are used for debugging, but as I just realized in my second comment the only place that actually reads the cl::opts is llc.cpp, so they are available in the gold plugin but will not have any effect. Same with clang. Seems like an accident that the cl::opt ended up in CommandFlags.h.

[tejohnson]

In D23050#503658, @MatzeB wrote:

In D23050#503625, @tejohnson wrote:

In D23050#503612, @MatzeB wrote:

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

I just realized that I can answer my own question: neither StartAfter nor StopAfter are used in gold-plugin.cpp so they can't have any effect in the gold plugin.

Sorry for the delayed response. I'm not personally familiar with -stop-after and -start-after. However, the gold-plugin will pass along any unrecognized options to the LLVM backend. So presumably a gold plugin user could specify them like -Wl,-plugin-opt,-start-after=pass-name.

I assume they are useful for debugging? Are they currently also supported for clang? E.g. clang -O2 -mllvm -start-after=pass-name ...?

Yes they are used for debugging, but as I just realized in my second comment the only place that actually reads the cl::opts is llc.cpp, so they are available in the gold plugin but will not have any effect. Same with clang. Seems like an accident that the cl::opt ended up in CommandFlags.h.

I see. In that case they should be in llc.cpp. But since the support for them is wired all the way through CodeGen, *should* it be supported for debugging via clang and gold-plugin?

[MatzeB]

In D23050#503698, @tejohnson wrote:

In D23050#503658, @MatzeB wrote:

In D23050#503625, @tejohnson wrote:

In D23050#503612, @MatzeB wrote:

I am mainly looking for confirmation that the gold plugin doesn't need -stop-after/-start-after in this review!

I just realized that I can answer my own question: neither StartAfter nor StopAfter are used in gold-plugin.cpp so they can't have any effect in the gold plugin.

Sorry for the delayed response. I'm not personally familiar with -stop-after and -start-after. However, the gold-plugin will pass along any unrecognized options to the LLVM backend. So presumably a gold plugin user could specify them like -Wl,-plugin-opt,-start-after=pass-name.

I assume they are useful for debugging? Are they currently also supported for clang? E.g. clang -O2 -mllvm -start-after=pass-name ...?

Yes they are used for debugging, but as I just realized in my second comment the only place that actually reads the cl::opts is llc.cpp, so they are available in the gold plugin but will not have any effect. Same with clang. Seems like an accident that the cl::opt ended up in CommandFlags.h.

I see. In that case they should be in llc.cpp. But since the support for them is wired all the way through CodeGen, *should* it be supported for debugging via clang and gold-plugin?

I guess there is a point to be made to move the option from llc.cpp to CodeGen/TargetPassConfig.cpp instead. But I guess there is no reason not to do the easier move to llc.cpp right now? We can do the move to TargetPassConfig.cpp another time in another patch. (I just wanted to get this done as a cleanup before I add -start-before/-stop-before flags because I need that for a test...).

[tejohnson]

Ok that seems reasonable, the move makes sense given the current option handling. It makes the current handling clearer.

llvm/trunk/include/llvm/CodeGen/CommandFlags.h

TrapFuncName("trap-func", cl::Hidden,
         cl::desc("Emit a call to trap function rather than a trap instruction"),
         cl::init(""));
 
 cl::opt<bool>
 UseCtors("use-ctors",
              cl::desc("Use .ctors instead of .init_array."),
              cl::init(false));
 
-cl::opt<std::string> StopAfter("stop-after",
-                            cl::desc("Stop compilation after a specific pass"),
-                            cl::value_desc("pass-name"),
-                                      cl::init(""));
-cl::opt<std::string> StartAfter("start-after",
-                          cl::desc("Resume compilation after a specific pass"),
-                          cl::value_desc("pass-name"),
-                          cl::init(""));
-
 cl::opt<bool> DataSections("data-sections",
                            cl::desc("Emit data into separate sections"),
                            cl::init(false));
 
 cl::opt<bool>
 FunctionSections("function-sections",
                  cl::desc("Emit functions into separate sections"),
                  cl::init(false));

llvm/trunk/tools/llc/llc.cpp

CompileTwice("compile-twice", cl::Hidden,
                           "manager and verify the result is the same."),
                  cl::init(false));
 
 static cl::opt<bool> DiscardValueNames(
     "discard-value-names",
     cl::desc("Discard names from Value (other than GlobalValue)."),
     cl::init(false), cl::Hidden);
 
+static cl::opt<std::string> StopAfter("stop-after",
+    cl::desc("Stop compilation after a specific pass"),
+    cl::value_desc("pass-name"), cl::init(""));
+
+static cl::opt<std::string> StartAfter("start-after",
+    cl::desc("Resume compilation after a specific pass"),
+    cl::value_desc("pass-name"), cl::init(""));
+
 namespace {
 static ManagedStatic<std::vector<std::string>> RunPassNames;
 
 struct RunPassOption {
   void operator=(const std::string &Val) const {
     if (Val.empty())
       return;
     SmallVector<StringRef, 8> PassNames;