LTO API: lto_codegen_optimize and lto_codegen_compile_optimized

Rafael Espíndola rafael.espindola at gmail.com
Mon Feb 2 11:33:48 PST 2015


Please clang-format the patch.
Please follow the llvm code style for new code (funcitonName, VariableName).

LGTM with that.


On 2 February 2015 at 14:07, Manman Ren <mren at apple.com> wrote:
> Hi all,
>
> This updated patch should address all the review comments.
>
> lto_api_version is added for ld64 to decide if the new api
> (lto_codegen_optimize and lto_codegen_compile_optimize) is available.
>
> Thanks,
> Manman
>
>
> On Jan 30, 2015, at 12:58 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com>
> wrote:
>
>
>
> On Jan 30, 2015, at 12:01 PM, Manman Ren <mren at apple.com> wrote:
>
> We currently have lto_codegen_compile that runs IR passes and codegen
> passes. If we dump the bitcode file after lto_codegen_compile, codegen
> passes can modify IR (one example is StackProtector pass).
> If we run “llc lto.opt.bc”, we will get assertion failure because llc runs
> StackProtector pass again.
>
> To enable dumping bitcode file right before running the codegen passes, this
> patch splits lto_codegen_compile in 2
> lto_codegen_optimize
> lto_codegen_compile_optimize
>
> Linker can choose to dump the bitcode file before calling
> lto_codegen_compile_optimize and we can debug the codegen passes from the
> dumped bitcode file.
>
> I initially had a different proposal, thanks to Rafael for suggesting this
> solution!
>
> Cheers,
> Manman
>
> <lto.patch>
>
>
> +kledzik (see discussion about lto_codegen_optimize() below).
>
> diff --git a/include/llvm-c/lto.h b/include/llvm-c/lto.h
> index 3f30d6d..46ebc79 100644
> --- a/include/llvm-c/lto.h
> +++ b/include/llvm-c/lto.h
> @@ -40,7 +40,7 @@ typedef bool lto_bool_t;
>  * @{
>  */
>
> -#define LTO_API_VERSION 11
> +#define LTO_API_VERSION 12
>
> /**
>  * \since prior to LTO_API_VERSION=3
> @@ -484,6 +484,27 @@ lto_codegen_compile(lto_code_gen_t cg, size_t* length);
> extern lto_bool_t
> lto_codegen_compile_to_file(lto_code_gen_t cg, const char** name);
>
>
> Please update `lto_codegen_compile_to_file()` and
> `lto_codegen_compile()` to be described in terms of the new functions.
>
>
> +/**
> + * Runs optimization for the merged module. Returns true on error.
> + *
> + * \since LTO_API_VERSION=12
> + */
> +extern lto_bool_t
> +lto_codegen_optimize(lto_code_gen_t cg);
>
>
> It's not clear to me how the linker can determine at runtime whether
> this function is available in the `dlopen()`'ed libLTO.dylib.
>
> IIRC, the ld64 checks for runtime-availability of the new API by calling
> the function, and assuming it doesn't exist if the function returns NULL
> (the return value of functions that don't exist at runtime).  Usually
> the flow is something like:
>
>        Result = nullptr;
>    #ifdef LTO_API_VERSION >= XYZ
>        Result = lto_new_api(...);
>        if (!Result)
>    #else
>        Result = lto_old_api(...)
>    #endif
>        // deal with Result
>
> That scheme doesn't work for functions that can return 0 as a valid
> result.
>
> Simply switching the error condition (0 on failure) won't work either,
> since then on error with the new API, you'd run the old API.
>
> I can see two possibilities:
>
> 1. Change `lto_codegen_optimize()` to return `1` on success and `2` on
>    failure.  Then `0` will mark that the API isn't available.
>
> 2. Add an `lto_api_version()` function that returns the runtime API
>    version.
>
> The latter option seems cleaner to me.
>
> @Nick: any thoughts on this?
>
> Then the linker code would look something like this:
>
>        Result = nullptr;
>    #ifdef LTO_API_VERSION >= XYZ
>        if (lto_api_version() >= XYZ)
>          Result = lto_new_api(...);
>        else
>    #else
>        Result = lto_old_api(...)
>    #endif
>        // deal with Result
>
> +
> +/**
> + * Generates code for the optimized merged module into one native object
> file.
> + * On success returns a pointer to a generated mach-o/ELF buffer and
> + * length set to the buffer size.  The buffer is owned by the
> + * lto_code_gen_t and will be freed when lto_codegen_dispose()
> + * is called, or lto_codegen_compile() is called again.
> + * On failure, returns NULL (check lto_get_error_message() for details).
>
>
> This comment should specify that it doesn't run IR-level optimizations.
>
> + *
> + * \since LTO_API_VERSION=12
> + */
> +extern const void*
> +lto_codegen_compile_optimized(lto_code_gen_t cg, size_t* length);
> +
>
> /**
>  * Sets options to help debug codegen bugs.
> diff --git a/include/llvm/LTO/LTOCodeGenerator.h
> b/include/llvm/LTO/LTOCodeGenerator.h
> index 0c9ce4a..ebba400 100644
> --- a/include/llvm/LTO/LTOCodeGenerator.h
> +++ b/include/llvm/LTO/LTOCodeGenerator.h
> @@ -117,6 +117,19 @@ struct LTOCodeGenerator {
>                       bool disableVectorization,
>                       std::string &errMsg);
>
> +  // Optimizes the merged module. Returns true on success.
> +  bool optimize(bool disableOpt,
> +                bool disableInline,
> +                bool disableGVNLoadPRE,
> +                bool disableVectorization,
> +                std::string &errMsg);
> +
> +  // Compiles the merged optimized module into a single object file. It
> brings the
> +  // object to a buffer, and returns the buffer to the caller. Return NULL
> if the
> +  // compilation was not successful.
> +  const void *compile_optimized(size_t *length,
> +                                std::string &errMsg);
>
>
> The function names here aren't in the C API, so they should follow the
> regular LLVM coding conventions.  Maybe `compileOptimized()`?
>
> +
>   void setDiagnosticHandler(lto_diagnostic_handler_t, void *);
>
>   LLVMContext &getContext() { return Context; }
> @@ -124,9 +137,8 @@ struct LTOCodeGenerator {
> private:
>   void initializeLTOPasses();
>
> -  bool generateObjectFile(raw_ostream &out, bool disableOpt, bool
> disableInline,
> -                          bool disableGVNLoadPRE, bool
> disableVectorization,
> -                          std::string &errMsg);
> +  bool compile_optimized(raw_ostream &out, std::string &errMsg);
> +  bool compile_optimized_to_file(const char** name, std::string &errMsg);
>
>
> Same here about function names.
>
>   void applyScopeRestrictions();
>   void applyRestriction(GlobalValue &GV, ArrayRef<StringRef> Libcalls,
>                         std::vector<const char *> &MustPreserveList,
> diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp
> index d937556..2fc9ee0 100644
> --- a/lib/LTO/LTOCodeGenerator.cpp
> +++ b/lib/LTO/LTOCodeGenerator.cpp
> @@ -201,12 +201,8 @@ bool LTOCodeGenerator::writeMergedModules(const char
> *path,
>   return true;
> }
>
> -bool LTOCodeGenerator::compile_to_file(const char** name,
> -                                       bool disableOpt,
> -                                       bool disableInline,
> -                                       bool disableGVNLoadPRE,
> -                                       bool disableVectorization,
> -                                       std::string& errMsg) {
> +bool LTOCodeGenerator::compile_optimized_to_file(const char** name,
> +                                                 std::string& errMsg) {
>   // make unique temp .o file to put generated object file
>   SmallString<128> Filename;
>   int FD;
> @@ -221,8 +217,7 @@ bool LTOCodeGenerator::compile_to_file(const char**
> name,
>   tool_output_file objFile(Filename.c_str(), FD);
>
>   bool genResult =
> -      generateObjectFile(objFile.os(), disableOpt, disableInline,
> -                         disableGVNLoadPRE, disableVectorization, errMsg);
> +      compile_optimized(objFile.os(), errMsg);
>   objFile.os().close();
>   if (objFile.os().has_error()) {
>     objFile.os().clear_error();
> @@ -241,15 +236,10 @@ bool LTOCodeGenerator::compile_to_file(const char**
> name,
>   return true;
> }
>
> -const void* LTOCodeGenerator::compile(size_t* length,
> -                                      bool disableOpt,
> -                                      bool disableInline,
> -                                      bool disableGVNLoadPRE,
> -                                      bool disableVectorization,
> -                                      std::string& errMsg) {
> +const void* LTOCodeGenerator::compile_optimized(size_t* length,
> +                                                std::string& errMsg) {
>   const char *name;
> -  if (!compile_to_file(&name, disableOpt, disableInline, disableGVNLoadPRE,
> -                       disableVectorization, errMsg))
> +  if (!compile_optimized_to_file(&name, errMsg))
>     return nullptr;
>
>   // read .o file into memory buffer
> @@ -272,6 +262,32 @@ const void* LTOCodeGenerator::compile(size_t* length,
>   return NativeObjectFile->getBufferStart();
> }
>
> +
> +bool LTOCodeGenerator::compile_to_file(const char** name,
> +                                       bool disableOpt,
> +                                       bool disableInline,
> +                                       bool disableGVNLoadPRE,
> +                                       bool disableVectorization,
> +                                       std::string& errMsg) {
> +  if (!optimize(disableOpt, disableInline, disableGVNLoadPRE,
> disableVectorization, errMsg))
>
>
> 80-column?
>
> +    return false;
> +
> +  return compile_optimized_to_file(name, errMsg);
> +}
> +
> +const void* LTOCodeGenerator::compile(size_t* length,
> +                                      bool disableOpt,
> +                                      bool disableInline,
> +                                      bool disableGVNLoadPRE,
> +                                      bool disableVectorization,
> +                                      std::string& errMsg) {
> +  if (!optimize(disableOpt, disableInline, disableGVNLoadPRE,
> +                       disableVectorization, errMsg))
>
>
> Whitespace looks strange here.  clang-format?
>
> +    return nullptr;
> +
> +  return compile_optimized(length, errMsg);
> +}
> +
> bool LTOCodeGenerator::determineTarget(std::string &errMsg) {
>   if (TargetMach)
>     return true;
> @@ -458,12 +474,11 @@ void LTOCodeGenerator::applyScopeRestrictions() {
> }
>
> /// Optimize merged modules using various IPO passes
> -bool LTOCodeGenerator::generateObjectFile(raw_ostream &out,
> -                                          bool DisableOpt,
> -                                          bool DisableInline,
> -                                          bool DisableGVNLoadPRE,
> -                                          bool DisableVectorization,
> -                                          std::string &errMsg) {
> +bool LTOCodeGenerator::optimize(bool DisableOpt,
> +                                bool DisableInline,
> +                                bool DisableGVNLoadPRE,
> +                                bool DisableVectorization,
> +                                std::string &errMsg) {
>   if (!this->determineTarget(errMsg))
>     return false;
>
> @@ -493,6 +508,22 @@ bool LTOCodeGenerator::generateObjectFile(raw_ostream
> &out,
>
>   PMB.populateLTOPassManager(passes, TargetMach);
>
> +  // Run our queue of passes all at once now, efficiently.
> +  passes.run(*mergedModule);
> +
> +  return true;
> +}
> +
> +bool LTOCodeGenerator::compile_optimized(raw_ostream &out,
> +                                         std::string &errMsg) {
> +  if (!this->determineTarget(errMsg))
> +    return false;
> +
> +  Module *mergedModule = IRLinker.getModule();
> +
> +  // Mark which symbols can not be internalized
> +  this->applyScopeRestrictions();
> +
>   PassManager codeGenPasses;
>
>   codeGenPasses.add(new DataLayoutPass());
> @@ -509,9 +540,6 @@ bool LTOCodeGenerator::generateObjectFile(raw_ostream
> &out,
>     return false;
>   }
>
> -  // Run our queue of passes all at once now, efficiently.
> -  passes.run(*mergedModule);
> -
>   // Run the code generator, and write assembly file
>   codeGenPasses.run(*mergedModule);
>
> diff --git a/tools/lto/lto.cpp b/tools/lto/lto.cpp
> index ec0372e..b4576cf 100644
> --- a/tools/lto/lto.cpp
> +++ b/tools/lto/lto.cpp
> @@ -289,6 +289,26 @@ const void *lto_codegen_compile(lto_code_gen_t cg,
> size_t *length) {
>                              sLastErrorString);
> }
>
> +bool lto_codegen_optimize(lto_code_gen_t cg) {
> +  if (!parsedOptions) {
> +    unwrap(cg)->parseCodeGenDebugOptions();
> +    lto_add_attrs(cg);
> +    parsedOptions = true;
> +  }
> +  return !unwrap(cg)->optimize(DisableOpt, DisableInline,
> +                               DisableGVNLoadPRE, DisableLTOVectorization,
> +                               sLastErrorString);
> +}
> +
> +const void *lto_codegen_compile_optimized(lto_code_gen_t cg, size_t
> *length) {
> +  if (!parsedOptions) {
> +    unwrap(cg)->parseCodeGenDebugOptions();
> +    lto_add_attrs(cg);
> +    parsedOptions = true;
> +  }
> +  return unwrap(cg)->compile_optimized(length, sLastErrorString);
> +}
> +
> bool lto_codegen_compile_to_file(lto_code_gen_t cg, const char **name) {
>   if (!parsedOptions) {
>     unwrap(cg)->parseCodeGenDebugOptions();
> diff --git a/tools/lto/lto.exports b/tools/lto/lto.exports
> index 8ba9d28..82277fb 100644
> --- a/tools/lto/lto.exports
> +++ b/tools/lto/lto.exports
> @@ -37,6 +37,8 @@ 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
> LLVMCreateDisasm
> LLVMCreateDisasmCPU
> LLVMDisasmDispose
>
>
>




More information about the llvm-commits mailing list