[LTO][Legacy] Add API to set result type to assembly

[w2yehia]

Summary

I'm trying to make libLTO (https://llvm.org/docs/LinkTimeOptimization.html#liblto) generate an assembly file instead of object format.
Right now, the --filetype=asm option has no effect in libLTO. This option is used by tools such as llvm-lto and llc.
libLTO directly interacts with LTOCodeGenerator, which does not use the option but has a method to override the default file type (CGFT_ObjectFile) .
My current solution is to add a new function to lto.h that sets the filetype to asm.

Another solution is to have one of the libLTO functions that create LTOCodeGenerator, and after the ParseCommandLineOptions has run, to query the --filetype option and call LTOCodeGenerator::setFileType if the option was specified (I haven't tried that, but shud work in theory).

[steven_wu]

Can you clarify your motivation for this? libLTO is used as an interface for linker and linkers, in general, do not understand assembly files.

[w2yehia]

In D88309#2295076, @steven_wu wrote:

Can you clarify your motivation for this? libLTO is used as an interface for linker and linkers, in general, do not understand assembly files.

Hi Steven, the object code codegen for the platform I'm using is not 100% functional, but the assembly generation is in much better shape, so we're taking the longer trip of calling the assembler to get the final .o file.

[steven_wu]

Note libLTO API needs to be stable so:

• If you need to add API, you need to bump API version, etc. Please refer to other commits that add APIs to see what needs to be done.
• If this is just a temporary workaround, I would resist make this change to bake in this API that will not be used in the future.

[tejohnson]

I'm not the right person to review this change, as we don't use the old LTO API at all. But I'll just add a quick note that there is a way to pass this through the new LTO API (lto::Config::CGFileType), and options to set it from lld/gold-plugin/llvm-lto2.

[hubert.reinterpretcast]

In D88309#2295142, @steven_wu wrote:

• If this is just a temporary workaround, I would resist make this change to bake in this API that will not be used in the future.

This seems to be conflating the immediate reason with the more general value of having such an interface. The interface that @tejohnson's earlier comment mentions presumably exists because there is value in having such an interface.

[steven_wu]

In D88309#2295281, @hubert.reinterpretcast wrote:

In D88309#2295142, @steven_wu wrote:

• If this is just a temporary workaround, I would resist make this change to bake in this API that will not be used in the future.

This seems to be conflating the immediate reason with the more general value of having such an interface. The interface that @tejohnson's earlier comment mentions presumably exists because there is value in having such an interface.

C++ interface is different from a libLTO C API which has additional requirement for compatibility. You can use the new C++ interface mentioned by @tejohnson or use the underlying c++ API for legacy API CG->setFileType() if you want.
I am fine with adding such a C API but I want to make sure you think through all the options, not just adding a new stable C API that is going to be thrown away.

[hubert.reinterpretcast]

In D88309#2295312, @steven_wu wrote:

I am fine with adding such a C API but I want to make sure you think through all the options, not just adding a new stable C API that is going to be thrown away.

We are using the C API because we need the stability. But yes, this points out that a linker using this would always be going through the assembly path even if the backend evolves past the point where that is a good idea.

[steven_wu]

In that case, please increment LTO_API_VERSION. If you think the API needs more documentation, please add to docs/LinkTimeOptimization.rst

[w2yehia]

@steven_wu what about the alternative solution where we make libLTO query the --filetype command line option. This will not require adding a new function to the API, but it does change the behavior of existing functions (lto_codegen_write_merged_modules, lto_codegen_compile, lto_codegen_optimize, lto_codegen_compile_optimized, lto_codegen_compile_to_file)

diff --git a/llvm/tools/lto/lto.cpp b/llvm/tools/lto/lto.cpp
index b680714..8251f7b 100644
--- a/llvm/tools/lto/lto.cpp
+++ b/llvm/tools/lto/lto.cpp
@@ -157,20 +157,23 @@
 
 // Convert the subtarget features into a string to pass to LTOCodeGenerator.
 static void lto_add_attrs(lto_code_gen_t cg) {
   LTOCodeGenerator *CG = unwrap(cg);
   auto MAttrs = codegen::getMAttrs();
   if (!MAttrs.empty()) {
     std::string attrs = join(MAttrs, ",");
     CG->setAttr(attrs);
   }
 
+  if (auto FT = codegen::getExplicitFileType())
+    CG->setFileType(FT.getValue());
+
   if (OptLevel < '0' || OptLevel > '3')
     report_fatal_error("Optimization level must be between 0 and 3");
   CG->setOptLevel(OptLevel - '0');
   CG->setFreestanding(EnableFreestanding);
 }

[steven_wu]

In D88309#2295403, @w2yehia wrote:

@steven_wu what about the alternative solution where we make libLTO query the --filetype command line option. This will not require adding a new function to the API, but it does change the behavior of existing functions (lto_codegen_write_merged_modules, lto_codegen_compile, lto_codegen_optimize, lto_codegen_compile_optimized, lto_codegen_compile_to_file)

diff --git a/llvm/tools/lto/lto.cpp b/llvm/tools/lto/lto.cpp
index b680714..8251f7b 100644
--- a/llvm/tools/lto/lto.cpp
+++ b/llvm/tools/lto/lto.cpp
@@ -157,20 +157,23 @@
 
 // Convert the subtarget features into a string to pass to LTOCodeGenerator.
 static void lto_add_attrs(lto_code_gen_t cg) {
   LTOCodeGenerator *CG = unwrap(cg);
   auto MAttrs = codegen::getMAttrs();
   if (!MAttrs.empty()) {
     std::string attrs = join(MAttrs, ",");
     CG->setAttr(attrs);
   }
 
+  if (auto FT = codegen::getExplicitFileType())
+    CG->setFileType(FT.getValue());
+
   if (OptLevel < '0' || OptLevel > '3')
     report_fatal_error("Optimization level must be between 0 and 3");
   CG->setOptLevel(OptLevel - '0');
   CG->setFreestanding(EnableFreestanding);
 }

That is not a bad idea but I think this is riskier than just adding a new API. Also, passing LTO option through -mllvm option (aka, through commanline option flags) is not officially supported.

I would go with a new C API.

[w2yehia]

Thanks Steven and Teresa.

Also, passing LTO option through -mllvm option (aka, through commanline option flags) is not officially supported.

Not sure what you mean by " through -mllvm option".
By the way, if you're using the GOLD plugin interface to invoke LTO, you just pass options thru -plugin-opt, no -mllvm. And in libLTO, you pass options through lto_codegen_debug_options C function.
Also, libLTO does query options defined in the LTO files (the function lto_add_attrs queries the -mattr option).

I'll take the new API approach then, unless there are objections from anyone else (shud we add more reviewers?)

llvm/include/llvm-c/lto.h

  * array.
  *
  * \since prior to LTO_API_VERSION=26
  */
 extern void lto_codegen_debug_options_array(lto_code_gen_t cg,
                                             const char *const *, int number);
 
 /**
+ * Sets the result filetype to assembly.
+ */
+extern void lto_codegen_emit_asm(lto_code_gen_t cg);
+
+/**
  * Initializes LLVM disassemblers.
  * FIXME: This doesn't really belong here.
  *
  * \since LTO_API_VERSION=5
  */
 extern void
 lto_initialize_disassembler(void);
 

llvm/tools/lto/lto.cpp

 void lto_codegen_debug_options_array(lto_code_gen_t cg,
                                      const char *const *options, int number) {
   SmallVector<StringRef, 4> Options;
   for (int i = 0; i < number; ++i)
     Options.push_back(options[i]);
   unwrap(cg)->setCodeGenDebugOptions(makeArrayRef(Options));
 }
 
+void lto_codegen_emit_asm(lto_code_gen_t cg) {
+  unwrap(cg)->setFileType(CGFT_AssemblyFile);
+}
+
 unsigned int lto_api_version() { return LTO_API_VERSION; }
 
 void lto_codegen_set_should_internalize(lto_code_gen_t cg,
                                         bool ShouldInternalize) {
   unwrap(cg)->setShouldInternalize(ShouldInternalize);
 }
 
 void lto_codegen_set_should_embed_uselists(lto_code_gen_t cg,

llvm/tools/lto/lto.exports

 lto_codegen_create
 lto_codegen_create_in_local_context
 lto_codegen_dispose
 lto_codegen_set_debug_model
 lto_codegen_set_pic_model
 lto_codegen_write_merged_modules
 lto_codegen_debug_options
 lto_codegen_debug_options_array
+lto_codegen_emit_asm
 lto_codegen_set_assembler_args
 lto_codegen_set_assembler_path
 lto_codegen_set_cpu
 lto_codegen_compile_to_file
 lto_codegen_optimize
 lto_codegen_compile_optimized
 lto_codegen_set_should_internalize
 lto_codegen_set_should_embed_uselists