[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment. Revised direction in https://reviews.llvm.org/D69638 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69356/new/ https://reviews.llvm.org/D69356 ___ cfe-commits mailing list cfe-

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment. In D69356#1727581 , @hubert.reinterpretcast wrote: > In D69356#1727484 , @beanz wrote: > > > We should not be adding more variables that are passed around by CMake's > > scope inheritance

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D69356#1727484 , @beanz wrote: > We should not be adding more variables that are passed around by CMake's > scope inheritance. Instead if we need to change this we should do it > correctly. Just to clarify, th

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D69356#1727381 , @beanz wrote: > (1) dead stripping support is useful independent of plugins, so it is > valuable to have that option be separate Okay. >> Just because the current implementation of plug-in sup

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In D69356#1727451 , @daltenty wrote: > The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools > which may have plugins and need to be built in a specific way (such as > avoiding deadstriping) if plugins are en

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment. > I'm actually opposed to that happening, on two fronts. > (2) we already have `LLVM_ENABLE_PLUGINS` why do we also need > `LLVM_SUPPORT_PLUGINS` seems like duplication and bad design. As I understand it LLVM_ENABLE_PLUGINS is a user-facing option which indicates whet

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment. In D69356#1726527 , @hubert.reinterpretcast wrote: > In D69356#1726354 , @beanz wrote: > > > If what you say is correct that "no dead strip" is harmful to plugins on > > some platforms it

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In D69356#1726527 , @hubert.reinterpretcast wrote: > For your second question: As the patch author indicated, the change to adjust > the behaviour on the affected platform is forthcoming. Your question seems > predicated upon that

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-29 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D69356#1726354 , @beanz wrote: > Can you please explain how the "no dead strip" semantic is harmful for > plug-in support, and how implying no dead strip for plugins is any different > from the current implement

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-29 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In D69356#1726336 , @hubert.reinterpretcast wrote: > Is there some documentation indicating these other use cases? If you have JIT'd code that needs to link against functions exposed in the process running the JIT you can't dead

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-29 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D69356#1726121 , @lhames wrote: > In D69356#1726074 , @beanz wrote: > > > ... It seems to me that maybe a more appropriate approach is that > > `LLVM_SUPPORT_PLUGINS` impl

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-29 Thread Lang Hames via Phabricator via cfe-commits
lhames added subscribers: beanz, lhames. lhames added a comment. In D69356#1726074 , @beanz wrote: > ... It seems to me that maybe a more appropriate approach is that > `LLVM_SUPPORT_PLUGINS` implies `LLVM_NO_DEAD_STRIP`, rather than conflating > the two

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-29 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. `LLVM_NO_DEAD_STRIP` isn't actually useful only for when plugins are enabled. It is also useful in a lot of contexts around building JIT host processes. The advantage of the old name was it specified exactly what it did (i.e. not dead strip code). It seems to me that may

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-25 Thread David Tenty via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG11c2a85db884: [NFC] Rename LLVM_NO_DEAD_STRIP (authored by daltenty). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69356/new/ https://reviews.llvm.org/D693

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-24 Thread Steven Wan via Phabricator via cfe-commits
stevewan accepted this revision. stevewan added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69356/new/ https://reviews.llvm.org/D69356 ___

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-24 Thread David Tenty via Phabricator via cfe-commits
daltenty updated this revision to Diff 226319. daltenty added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Address comments round 1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69356/new/ https://reviews.llvm.o