[patch] Produce .weak_def_can_be_hidden for some linkonce_odr values

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Oct 28 21:14:06 PDT 2013


>
> -      if (Linkage != GlobalValue::LinkOnceODRAutoHideLinkage)
> +
> +      bool CanBeHidden = Linkage ==
> GlobalValue::LinkOnceODRAutoHideLinkage;
> +
> +      if (!CanBeHidden && Linkage == GlobalValue::LinkOnceODRLinkage) {
> +        if (GV->hasUnnamedAddr()) {
> +          CanBeHidden = true;
> +        } else {
> +          GlobalStatus GS;
> +          if (!GlobalStatus::analyzeGlobal(GV, GS) && !GS.IsCompared)
>
> Ah, really? Put another way, why should we pay this penalty *here*? This is
> something we can compute in the mid-level optimizer and should compute in
> the mid-level optimizer, and we shouldn't waste our time second-guessing it
> here.
>
> I assume the real problem is that we don't do a second run of globalopt at
> the end of the TU to mark more things with unnamed_addr?

No, the problem is that there are more can_be_hidden than there are
unnamed_addr.

The semantics of unnamed_addr is that we know the address is not
significant. That is true even if its address *is* taken. For example,
a virtual function in a vtable. Given those semantics, GlobalOpt
cannot mark a function unnamed_addr (unless it is already local).
There was a patch/discussion to add a notaddrtaken flag, but right now
the only way to know that the address of a globalvariable is not used
is to analyze its uses.

> How much do we lose
> if you don't do this recomparison, is that tolerable? I realize that getting
> a second run of globalopt at the end is going to be time consuming to prove
> worthwhile, but I really think it's the right way to go.

With a notaddrtaken attribute the run of GlobalOpt we have would be
sufficient. This check is at the very end of the compilation.  I will
benchmark a build without the GlobalStatus::analyzeGlobal and report
as soon as the bootstrap finishes.



> +            CanBeHidden = true;
> +        }
> +      }
> +
> +      if (!CanBeHidden)
>
> Everything else about the patch LGTM.
>
>> I just tested this with the linker in Xcode 5 and the results are:
>>
>> $ nm  build-with-master/bin/clang-3.4  | grep  ' T ' | wc
>>
>>    21053 63159 1790901
>>
>> $ nm  build-with-patch/bin/clang-3.4  | grep  ' T ' | wc
>>
>>    19049   57147 1547425
>
>
> Nice!!
>
> Nick


Cheers,
Rafael



More information about the llvm-commits mailing list