[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

Tue Oct 29 20:21:14 PDT 2019

hubert.reinterpretcast added a comment.

In D69356#1726354 <https://reviews.llvm.org/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 implementation.

For your first question: AIX performs garbage collection by default. Disabling garbage collection does not work (regardless of plug-in support). The system libraries contain references to undefined symbols from sections that are normally garbage collected.
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 not happening.

> This patch does not change behavior, it just makes the option name more confusing my linking disabling dead stripping to plugins.

Just because the current implementation of plug-in support is by disabling dead strip does not make it a great reason for "enabling plug-in support" to be called `LLVM_NO_DEAD_STRIP`.

> If what you say is correct that "no dead strip" is harmful to plugins on some platforms it seems like this rename was a step in the direction of more confusion.

Only when focusing on the implementation and not the intent.

If you believe there should be a way to request "no dead strip" regardless of what the user intended it for (and whether it would work for their intended use case), then I guess we can keep such an option. In such a case, then for some platforms, requesting that an executable be linked such that plug-in support is enabled will imply "no dead strip" (as you suggested). Perhaps the more accurate intent for both the JIT host process and the plug-in support case is that the executable is expected to make symbols available for run-time linking. Maybe `LLVM_EXPORT_SYMBOLS_FROM_EXEC` is a better name?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69356/new/

https://reviews.llvm.org/D69356