[llvm-commits] Fix internalize in the case where no symbols are needed

[Duncan Sands]

Hi Rafael,

On 26/10/12 15:02, Rafael Espíndola wrote:
>> while this patch looks OK to me, what do you think about removing the
>> special
>> role of "main"?  Is there any reason why users of this pass (eg the "opt"
>> tool)
>> can't just pass in "main" if they don't want "main" to be internalized?
>
> I would *love* to do that :-)
>
> Is the attached patch OK?

> diff --git a/include/llvm-c/Transforms/IPO.h b/include/llvm-c/Transforms/IPO.h
> index 4480780..b16aec3 100644
> --- a/include/llvm-c/Transforms/IPO.h
> +++ b/include/llvm-c/Transforms/IPO.h
> @@ -62,7 +62,7 @@ void LLVMAddPruneEHPass(LLVMPassManagerRef PM);
>  void LLVMAddIPSCCPPass(LLVMPassManagerRef PM);
>
>  /** See llvm::createInternalizePass function. */
> -void LLVMAddInternalizePass(LLVMPassManagerRef, unsigned AllButMain);
> +void LLVMAddInternalizePass(LLVMPassManagerRef);

This changes the C API, which is a no-no.  I suggest instead leaving the
AllButMain parameter here, and internally pushing "main" onto the list
of exceptional symbols if someone passes 'true' for AllButMain.  This isn't
*exactly* the same as before (it internalizes everything, rather than
internalizing nothing, if there is no "main"), but it's hopefully close enough
to be OK.  This change maybe should be documented in the release notes.

>
>  /** See llvm::createStripDeadPrototypesPass function. */
>  void LLVMAddStripDeadPrototypesPass(LLVMPassManagerRef PM);
> --- a/lib/Transforms/IPO/Internalize.cpp
> +++ b/lib/Transforms/IPO/Internalize.cpp
> @@ -7,9 +7,9 @@
>  //
>  //===----------------------------------------------------------------------===//
>  //
> -// This pass loops over all of the functions in the input module, looking for a
> -// main function.  If a main function is found, all other functions and all
> -// global variables with initializers are marked as internal.
> +// This pass loops over all of the functions and variables in the input module.
> +// If the function or variable is not one of the exceptions given to the pass,

As "exceptions" is kind of overloaded, how about: If the function or variable
is not in the list of external names given to the pass,

> +// it is marked as internal.
>  //
>  //===----------------------------------------------------------------------===//
>
> diff --git a/lib/Transforms/IPO/PassManagerBuilder.cpp b/lib/Transforms/IPO/PassManagerBuilder.cpp
> index 1d8f1e5..b39433f 100644
> --- a/lib/Transforms/IPO/PassManagerBuilder.cpp
> +++ b/lib/Transforms/IPO/PassManagerBuilder.cpp
> @@ -246,7 +246,7 @@ void PassManagerBuilder::populateLTOPassManager(PassManagerBase &PM,
>    // for a main function.  If main is defined, mark all other functions
>    // internal.
>    if (Internalize)
> -    PM.add(createInternalizePass(true));
> +    PM.add(createInternalizePass());

This changes the behaviour of -std-link-opts in a pretty big way (probably
breaks the nightly testsuite, which uses -std-link-opts).  How about just
pushing "main" into the list of external names here?

>    // Propagate constants at call sites into the functions they call.  This
>    // opens opportunities for globalopt (and inlining) by substituting function

Ciao, Duncan.