[PATCH] D154507: [NVPTX] Apply global var demotion to private symbols

Tue Jul 18 02:49:59 PDT 2023

qcolombet added a comment.

Hi @jdoerfert,

In D154507#4487631 <https://reviews.llvm.org/D154507#4487631>, @jdoerfert wrote:

> In D154507#4487519 <https://reviews.llvm.org/D154507#4487519>, @mehdi_amini wrote:
>
>> I'm a bit confused @jdoerfert : are there two different uses of the word "demoted" here? Seems like you're referring to a transformation that turns a global into an alloca, whereas here is it just about "demoting" the linkage type of the global?
>
> I do not see the linkage being changed, if so, to what? Local (=private or internal) is already as "good" as it gets.
>
> Given the test result, and the comment ("Find out if a global variable can be demoted to local scope."), it looks to me as if the scope is changed, thus where the variable is "declared/visible".
> That said, I was expecting (w/o knowing for sure), that a variable scoped in a function cannot be accessed by other functions. If that is wrong, the first part of my comment is mood.

I think a variable scoped in a function can be accessed by other functions. Meaning, the symbol may not be visible, but the address pointed by this symbol can be accessed (i.e., it is okay to escape), because the allocations are handled at a higher level (the driver).
@guraypp do you have more details on this?

> I still believe we don't want these special casings in our backends, but that is for later. If the scope doesn't preclude access, this is fine.

@guraypp could you explain why this optimization is useful?
I don't remember. I would expect the visibility doesn't matter as long as the (meta)data around what needs to be allocated before a kernel launch is accurate, but maybe this info is derived from the symbols visibility?

> EDIT:
> I looked at the PTX spec online but could not figure it out right away. One can always try to escape the address, recurse, check that the address is still the same and that outside accesses are visible as they should be.

Looking at how `usedInOneFunc` is implemented, you're right that the address could escape. That being said, I don't think this is an issue since the memory allocation is handled at the kernel level. Anyhow, this is orthogonal to this patch IMHO.

Cheers,
-Quentin

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154507