[llvm] [ElimAvailExtern] Add an option to allow to convert global variables in a specified address space to local (PR #144287)

[Teresa Johnson via llvm-commits]

teresajohnson wrote:

> > Except that the PR description indicates that this is only for the case that the function was imported and privatized:
> 
> The description just indicates the motivation and a use of this option, but it can be definitely used for other purposes but it would be more target dependent.
> 
> > Is it ever safe to do this for an available externally variable not in an available externally function that is being privatized?
> 
> If we are talking about from AMDGPU's perspective, there would be no available externally function that is not being privatized in the first place because no external function call can be supported.

Ok. The reason I ask all these questions is that it is fundamentally more dangerous to privatize a copy of a global variable, because multiple copies of a read-write variable may end up reading different values (versus a function which is always read-only). At the least, can you add a comment about the intended use and the risk of using this improperly? Is it possible to guard by the target triple or something indicating that it is compiling for AMDGPU?


> If we think from a target agnostic perspective, I'd say it'd depend on the target about how to interpret the AS of a global variable and there is no hard relation between an externally available function and an externally available global variable.
> 
> > (BTW I see that this was already merged, which was a little premature as there was outstanding questions from a reviewer who hadn't yet approved - it's good to at least give it a day or so to see if there is follow on, I didn't have a chance to go back and look at this a second time yesterday)
> 
> Understood. I thought you would not have new response since I did wait for half a day. I'll take a better practice next time.

No worries, thanks.



https://github.com/llvm/llvm-project/pull/144287