[patch] prototype of storing MCSymbolData directly in the MCAssembler

Rafael EspĂ­ndola rafael.espindola at gmail.com
Sun Apr 27 11:16:18 PDT 2014


On 24 April 2014 13:28, David Blaikie <dblaikie at gmail.com> wrote:
> TL,DR; Playing around with data structures, not high priority - read
> on only if you're bored/curious.

check :-)

> So when looking at the MCSymbolData leak in MCAsmStreamer I went
> looking at wyh MCSymbolData was an ilist_node and I found the iplist
> in MCAssembler.
>
> This data isn't polymorphic, particularly large, nor requires the
> ability to insert/remove/iterate from bare MCSymbolData objects (ie:
> doesn't need the intrusive property nor the indirection of pointers).
>
> So I prototyped the semi-obvious change of replacing these:
>
> iplist<MCSymbolData> Symbols;
> DenseMap<const MCSymbol*, MCSymbol*> SymbolMap;
>
> with this:
>
> std::unordered_map<const MCSymbol *, MCSymbolData> SymbolMap;
>
> (hmm, now that I reread the "FIXME: avoid this indirection" I guess my
> patch doesn't address that comment, which is presumably about avoiding
> the need for the map entirely - which is a fair question: why isn't
> the MCSymbolData just stored in the MCSymbol?)

I think the distinction exists just so the asm streamer doesn't have
to create MCSymbolData. I think the two best implementations probably
are

* Have MCSymbol include a pointer to MCSymbolData. The pointer is
always null when printing assembly.
* Just merge the two.

The second option is a bit more efficient in the cases we actually
care about (producing object files). The first one is a bit cleaner. I
would probably use the first one at least for now.

Cheers,
Rafael



More information about the llvm-commits mailing list