LTO API: lto_codegen_optimize and lto_codegen_compile_optimized

[Nick Kledzik]

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).

See if I understand this.  Instead of calling lto_codegen_compile() the linker would call lto_codegen_optimize() then lto_codegen_compile_optimized().  But the linker only needs to use the two new calls if it plans to write out the bitcode (-r or -save-temps) via lto_codegen_write_merged_modules().

If that is the only usage, why not combine the three into one function:

  extern const void*
  lto_codegen_compile_and_write_bitcode(lto_code_gen_t cg, const char *path, size_t* length);


> 
>> 
>> +/**
>> + * 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
If we needed to support functions that return 0 as a valid result, then yes, we’d need something like this.

-Nick