[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D92633#2433999 <https://reviews.llvm.org/D92633#2433999>, @tmsriram wrote:

> In D92633#2433979 <https://reviews.llvm.org/D92633#2433979>, @MaskRay wrote:
>
>> In D92633#2433108 <https://reviews.llvm.org/D92633#2433108>, @tmsriram wrote:
>>
>>> You said : "The name -mpie-copy-relocations is misleading [1] and does not capture the idea that this option can actually apply to all of -fno-pic,-fpie, ..."
>>>
>>> Could you please clarify why this option needs to apply to -fno-pic?  Here is what I tried with trunk clang:
>>
>> If the user wants to guarantee no copy relocations in -fno-pic code, they can theoretically apply -fno-direct-access-external-data to use a GOT indirection.
>> This is not implemented, though.
>>
>>> extern int var;
>>> int get() { return var; }
>>>
>>> $ clang -S foo.c -o -
>>> ....
>>> 	movl	var, %eax
>>> 	popq	%rbp
>>> ...
>>>
>>> With -fno-pic,  this will never need to use -mpie-copy-relocations at all, so this case is not relevant right?  Did I miss anything?
>>
>> -fno-pic code can only be used with -no-pie links (position-dependent executables) If var is not defined in the linked executable, it will have a copy relocation.
>
> Thanks for explaining.  I know that by default (i.e. no-pic and no-pie), copy relocations will be used for external data accesses.  So, you are saying that you are adding a mechanism to disable copy relocations for the no-pic/no-pie case too. Is there a need for this, purely a question.  I know that copy relocations are frowned upon so maybe there was a feature request. If so, citing that would make it more clear. Thanks.

I don't know of a feature request.

> Is there a need for this, purely a question.

A weak need: more to make the behavior more align with the option name and perhaps align with potential GCC behavior (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112).
It can also be argued that this can be used to remove a special case for ppc64 ELFv2, which essentially defaults to -fno-direct-access-external-data for -fno-pic (they prefer to not have copy relocations)

Supporting -fno-direct-access-external-data for -fno-pic is not difficult: a one/two line change to `TargetMachine::shouldAssumeDSOLocal` refining `if (!(GV && GV->isThreadLocal()) && RM == Reloc::Static)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92633