This is an archive of the discontinued LLVM Phabricator instance.

ExpressionParser: only force link MCJIT when needed
ClosedPublic

Authored by compnerd on May 2 2019, 5:07 PM.

Details

Summary

This was added to support FreeBSD. The inclusion of this header increases the
size of lldb-server due to MCJIT being forcefully preserved. Conditionalise
the inclusion to shared builds of LLVM which will allow for MCJIT to be stripped
if unnecessary when performing static linking of tools. This shaves off ~28% of
the binary size for lldb-server when linked with gold using
-ffunction-sections and -fdata-sections.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

compnerd created this revision.May 2 2019, 5:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2019, 5:07 PM

gold-2.27, clang 6.0.0, -ffunction-sections, -fdata-sections, -DLLVM_ENABLE_UNWIND_TABLES=NO, MinSizeRel:

BEFORE

bin/lldb-server  :
section                       size       addr
.interp                         28    4194928
.note.ABI-tag                   32    4194956
.dynsym                      11208    4194992
.gnu.hash                      104    4214304
.gnu.version                   934    4214408
.gnu.version_r                 608    4215344
.init                           26    4224736
.plt                          5520    4224768
.text                      4213455    4230288
.fini                            9    8443744
.rodata                    7685708    8443760
.eh_frame                     1660   16129472
.eh_frame_hdr                  332   16131132
.tbss                           41   16136640
.jcr                             8   16136640
.fini_array                      8   16136648
.init_array                    432   16136656
.data.rel.ro               1220808   16137088
.dynamic                       640   17357896
.got                           280   17358544
.got.plt                      2776   17358824
.tm_clone_table                  0   17361600
.data                        35072   17361600
.bss                         45632   17396672
.comment                       443          0
.note.gnu.gold-version          28          0
*COM*                            0          0
Total                     13225792

AFTER

bin/lldb-server  :
section                       size       addr
.interp                         28    4194928
.note.ABI-tag                   32    4194956
.dynsym                      11184    4194992
.gnu.hash                      104    4214264
.gnu.version                   932    4214368
.gnu.version_r                 608    4215300
.init                           26    4223928
.plt                          5072    4223968
.text                      2647408    4229040
.fini                            9    6876448
.rodata                    5960320    6876464
.eh_frame                     1540   12836784
.eh_frame_hdr                  292   12838324
.tbss                           41   12845952
.jcr                             8   12845952
.fini_array                      8   12845960
.init_array                    416   12845968
.data.rel.ro               1132392   12846384
.dynamic                       640   13978776
.got                           200   13979424
.got.plt                      2552   13979624
.tm_clone_table                  0   13982176
.data                        33136   13982176
.bss                         44914   14015312
.comment                       443          0
.note.gnu.gold-version          28          0
*COM*                            0          0
Total                      9842333
labath requested changes to this revision.May 3 2019, 2:58 AM

Did you run the test suite with this change? I get about 200 failures (non-shlib build), which is not surprising as without this lldb will not actually have a jit engine to evaluate the expressions with.

The best thing to do here would be make sure lldb-server never even pulls the ClangExpressionParser library into its dependency graph, but in the current state of the graph, that would likely take the rest of this year. So given the simplicity of this change, and the amount of code it saves us, I think we could settle for something slightly sub-optimal, but which will still produce the desired effect.

What I propose is to move this header into the SystemInitializerFull class. That should still result in MCJIT being linked into liblldb (as it should be), but exclude it from lldb-server. The SystemInitializerFull class is a slightly odd place for this, but *only* slightly, because it's where we do the rest of llvm initialization (llvm::InitializeAllTargets() and friends), and the nature of this header is very initialization-like.

This revision now requires changes to proceed.May 3 2019, 2:58 AM
compnerd updated this revision to Diff 198020.May 3 2019, 8:33 AM

Move the inclusion as suggested.

labath accepted this revision.May 3 2019, 9:07 AM

Thanks. Let's give that a shot.

This revision is now accepted and ready to land.May 3 2019, 9:07 AM
clayborg accepted this revision.May 3 2019, 4:16 PM
compnerd closed this revision.May 3 2019, 4:17 PM

SVN r359944