<div dir="ltr"><div class="gmail_extra"><div><div class="gmail_signature" data-smartmail="gmail_signature">On Thu, Mar 8, 2018 at 10:17 AM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> +static void readCallGraph(MemoryBufferRef MB) {<br>
> +  // Build a map from symbol name to section<br>
> +  DenseMap<StringRef, InputSectionBase *> SymbolSection;<br>
> +  for (InputFile *File : ObjectFiles)<br>
> +    for (Symbol *Sym : File->getSymbols())<br>
> +      if (auto *D = dyn_cast<Defined>(Sym))<br>
> +        if (auto *IS = dyn_cast_or_null<<wbr>InputSectionBase>(D->Section))<br>
> +          SymbolSection[D->getName()] = IS;<br>
> +<br>
> +  std::vector<StringRef> Lines = args::getLines(MB);<br>
> +  for (StringRef L : Lines) {<br>
> +    SmallVector<StringRef, 3> Fields;<br>
> +    L.split(Fields, ' ');<br>
> +    if (Fields.size() != 3)<br>
> +      fatal("parse error");<br>
> +    uint64_t Count;<br>
> +    if (!to_integer(Fields[2], Count))<br>
> +      fatal("parse error");<br>
> +    InputSectionBase *FromSec = SymbolSection.lookup(Fields[0]<wbr>);<br>
> +    InputSectionBase *ToSec = SymbolSection.lookup(Fields[1]<wbr>);<br>
> +    if (FromSec && ToSec)<br>
> +      Config->CallGraphProfile[std::<wbr>make_pair(FromSec, ToSec)] += Count;<br>
<br>
</span>Should this be using FromSec->Repl, ToSec->Repl to handle ICF? If so<br>
please update the code and add a new test (the one in the patch is<br>
getting big).<br></blockquote><div><br></div><div>I'll look into this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +  }<br>
> +}<br>
<br>
With --symbol-ordering-file we produce warnings if, for example, the<br>
symbol is found to be absolute. The same issue applies here, no? Can you<br>
refactor the code so that we print the same warnings for<br>
--call-graph-ordering-file?<br></blockquote><div><br></div><div>Given how the call graph files are likely to be produced I would expect warning on that would just be noise.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> +using ClusterIndex = std::ptrdiff_t;<br>
> +using SectionIndex = std::ptrdiff_t;<br>
> +using EdgeIndex = std::ptrdiff_t;<br>
<br>
</span>Does this means that we use 64 bit integers for indexes? Why?<br></blockquote><div><br></div><div>On a 64 bit platform, yes.  It's used because it can index all sections on whatever platform it's on.  It's signed because the only reason to use unsigned types is if you need the range, are bit twiddling, or need modular arithmetic.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> +// Used for calculating an comparing density.  Use soft-float for determinism.<br>
> +struct Double : APFloat {<br>
<br>
</span>This used to be a problem with x87, but do we support a system today<br>
where this is a problem?<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div><div class="gmail_extra">We don't use float/double anywhere else in the compiler.  I do not have high confidence that all supported versions of clang, gcc, and msvc for all supported targets will have bit identical behavior here.</div><div class="gmail_extra"><br></div><div class="gmail_extra">- Michael Spencer</div><div class="gmail_extra"><br></div></div>