[PATCH] LTO: add API to set strategy for -internalize

[Justin Bogner]

"Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
> Nick: the new patch (attached below) is updated as per our
> conversation.  Thanks for the review!
>
> Can anyone review the LLVM side?

LGTM with a couple minor things:

> diff --git a/lib/Transforms/IPO/Internalize.cpp b/lib/Transforms/IPO/Internalize.cpp
> index dae69ce..6c5128e 100644
> --- a/lib/Transforms/IPO/Internalize.cpp
> +++ b/lib/Transforms/IPO/Internalize.cpp
> @@ -47,48 +47,50 @@ APIFile("internalize-public-api-file", cl::value_desc("filename"),
>  
>  // APIList - A list of symbols that should not be marked internal.
>  static cl::list<std::string>
>  APIList("internalize-public-api-list", cl::value_desc("list"),
>          cl::desc("A list of symbol names to preserve"),
>          cl::CommaSeparated);
>  
>  namespace {
>    class InternalizePass : public ModulePass {
>      std::set<std::string> ExternalNames;
> +    bool OnlyHidden;
>    public:
>      static char ID; // Pass identification, replacement for typeid
> -    explicit InternalizePass();
> -    explicit InternalizePass(ArrayRef<const char *> ExportList);
> +    explicit InternalizePass(bool OnlyHidden = false);
> +    explicit InternalizePass(ArrayRef<const char *> ExportList, bool OnlyHidden);

Why give one of these a default and not the other?

> @@ -99,21 +101,26 @@ void InternalizePass::LoadFile(const char *Filename) {
>    }
>    while (In) {
>      std::string Symbol;
>      In >> Symbol;
>      if (!Symbol.empty())
>        ExternalNames.insert(Symbol);
>    }
>  }
>  
>  static bool shouldInternalize(const GlobalValue &GV,
> -                              const std::set<std::string> &ExternalNames) {
> +                              const std::set<std::string> &ExternalNames,
> +                              bool OnlyHidden) {
> +  // Only internalize hidden symbols.
> +  if (OnlyHidden && !GV.hasHiddenVisibility())
> +    return false;
> +

The comment is confusing here. It might read better as "Check if we're
only internalizing hidden symbols", but I'm not convinced it adds
anything to the code anyway.