Duncan P. N. Exon Smith
2015-May-13 21:48 UTC
[LLVMdev] RFC: Merge MCSymbol with MCSymbolData, optimizing for object file emission
Right now MC is optimized for emitting assembly, but as Rafael pointed out to me over IRC the optimized path should be emitting object files. This WIP patch moves MCSymbolData into MCSymbol.h and makes it a field inside MCSymbol. This eliminates a pointer from MCSymbolData back to MCSymbol, the DenseMap<const MCSymbol *, MCSymbolData *> in MCAssembler, and converts the iplist in MCAssembler into a std::vector (along with some churn to pass around MCSymbols instead of MCSymbolData). As a result, during object emission we save ~6 pointers per MCSymbol, eliminate lookup indirection between MCSymbol and MCSymbolData, and avoid allocation overhead for MCSymbolData. I've measured ~4% memory savings on `llc` with this patch (using the same -flto -g input as r236642 and r236629 before it), dropping memory usage from 1058 MB down to 1017 MB. Before I clean this up and commit it (obviously in more incremental patches (and I'll do the same cleanup for MCSection)), I wanted to confirm the direction. -------------- next part -------------- A non-text attachment was scrubbed... Name: symbol-data-wip.patch Type: application/octet-stream Size: 89529 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150513/3cae1543/attachment.obj>
David Blaikie
2015-May-13 22:10 UTC
[LLVMdev] RFC: Merge MCSymbol with MCSymbolData, optimizing for object file emission
On Wed, May 13, 2015 at 2:48 PM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> Right now MC is optimized for emitting assembly, but as Rafael pointed > out to me over IRC the optimized path should be emitting object files. > > This WIP patch moves MCSymbolData into MCSymbol.h and makes it a > field inside MCSymbol. This eliminates a pointer from MCSymbolData back > to MCSymbol, the DenseMap<const MCSymbol *, MCSymbolData *> in > MCAssembler, and converts the iplist in MCAssembler into a std::vector > (along with some churn to pass around MCSymbols instead of > MCSymbolData). As a result, during object emission we save ~6 pointers > per MCSymbol, eliminate lookup indirection between MCSymbol and > MCSymbolData, and avoid allocation overhead for MCSymbolData. > > I've measured ~4% memory savings on `llc` with this patch (using the > same -flto -g input as r236642 and r236629 before it), dropping memory > usage from 1058 MB down to 1017 MB. > > Before I clean this up and commit it (obviously in more incremental > patches (and I'll do the same cleanup for MCSection)), I wanted to > confirm the direction. >I've no real idea if it's the right direction - but I was certainly mystified by the various indirections here a while back (when implementing DWARF compression) & made some (failed) attempts to reconcile them. You mentioned this is is a matter of optimizing for object output rather than asm output - do you have stats on how much this hurts asm output performance, then?> > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150513/e6193fff/attachment.html>
Duncan P. N. Exon Smith
2015-May-13 22:25 UTC
[LLVMdev] RFC: Merge MCSymbol with MCSymbolData, optimizing for object file emission
> On 2015 May 13, at 15:10, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Wed, May 13, 2015 at 2:48 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > Right now MC is optimized for emitting assembly, but as Rafael pointed > out to me over IRC the optimized path should be emitting object files. > > This WIP patch moves MCSymbolData into MCSymbol.h and makes it a > field inside MCSymbol. This eliminates a pointer from MCSymbolData back > to MCSymbol, the DenseMap<const MCSymbol *, MCSymbolData *> in > MCAssembler, and converts the iplist in MCAssembler into a std::vector > (along with some churn to pass around MCSymbols instead of > MCSymbolData). As a result, during object emission we save ~6 pointers > per MCSymbol, eliminate lookup indirection between MCSymbol and > MCSymbolData, and avoid allocation overhead for MCSymbolData. > > I've measured ~4% memory savings on `llc` with this patch (using the > same -flto -g input as r236642 and r236629 before it), dropping memory > usage from 1058 MB down to 1017 MB. > > Before I clean this up and commit it (obviously in more incremental > patches (and I'll do the same cleanup for MCSection)), I wanted to > confirm the direction. > > I've no real idea if it's the right direction - but I was certainly mystified by the various indirections here a while back (when implementing DWARF compression) & made some (failed) attempts to reconcile them. > > You mentioned this is is a matter of optimizing for object output rather than asm output - do you have stats on how much this hurts asm output performance, then? >I didn't do any measurements for asm. It should have increased memory usage of `sizeof(MCSymbolData)` for each symbol. That's an extra 5 pointers, so it should be around 3.3% memory usage increase (5/6 of the decrease for objects).
Duncan P. N. Exon Smith
2015-May-15 23:18 UTC
[LLVMdev] RFC: Merge MCSymbol with MCSymbolData, optimizing for object file emission
+llvm-commits, bcc:llvmdev> On 2015-May-13, at 14:48, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > > Right now MC is optimized for emitting assembly, but as Rafael pointed > out to me over IRC the optimized path should be emitting object files. > > This WIP patch moves MCSymbolData into MCSymbol.h and makes it a > field inside MCSymbol. This eliminates a pointer from MCSymbolData back > to MCSymbol, the DenseMap<const MCSymbol *, MCSymbolData *> in > MCAssembler, and converts the iplist in MCAssembler into a std::vector > (along with some churn to pass around MCSymbols instead of > MCSymbolData). As a result, during object emission we save ~6 pointers > per MCSymbol, eliminate lookup indirection between MCSymbol and > MCSymbolData, and avoid allocation overhead for MCSymbolData. > > I've measured ~4% memory savings on `llc` with this patch (using the > same -flto -g input as r236642 and r236629 before it), dropping memory > usage from 1058 MB down to 1017 MB. > > Before I clean this up and commit it (obviously in more incremental > patches (and I'll do the same cleanup for MCSection)), I wanted to > confirm the direction. > > <symbol-data-wip.patch>I went ahead and committed r237486 and r237487, which converted the iplist in MCAssembler into a std::vector, and I've started to clean up for commit. (More to do still...) The one patch (so far) I want pre-commit review on is 0002 -- i.e., is this direction okay? -- since the rest are just natural fallout. I've attached them for reference anyway so you can see where I'm headed (0001 is just moving code as prep for 0002). I know Rafael's on board -- he nerd-sniped me into this -- but he's on vacation ;). Any chance of a go-ahead from someone else that's spent some time in MC? -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-MC-Move-MCSymbolData-to-MCSymbol.h-NFC.patch Type: application/octet-stream Size: 8721 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-MC-Merge-MCSymbol-and-MCSymbolData.patch Type: application/octet-stream Size: 6164 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0001.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0003-MC-Change-MCAssembler-Symbols-to-store-MCSymbol-NFC.patch Type: application/octet-stream Size: 5480 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0002.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0004-MC-Change-MCFragment-Atom-to-an-MCSymbol-NFC.patch Type: application/octet-stream Size: 12988 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0003.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0005-MC-Use-MCSymbol-in-MCObject-IsSymbolRefDifferenceFul.patch Type: application/octet-stream Size: 8646 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0004.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0006-MC-Make-MCSymbolData-Symbol-private.patch Type: application/octet-stream Size: 66033 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0005.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0007-MC-Simplify-MCSymbolData-initialization-and-remove-M.patch Type: application/octet-stream Size: 5974 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0006.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: all.patch Type: application/octet-stream Size: 87699 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150515/3c2b54ec/attachment-0007.obj>