[PATCH] D115661: [clang][amdgpu] - Choose when to promote VarDecl to address space 4.

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 14 14:26:01 PST 2021


yaxunl added a comment.

In D115661#3193157 <https://reviews.llvm.org/D115661#3193157>, @arsenm wrote:

> In D115661#3193152 <https://reviews.llvm.org/D115661#3193152>, @yaxunl wrote:
>
>> In D115661#3192983 <https://reviews.llvm.org/D115661#3192983>, @estewart08 wrote:
>>
>>> In D115661#3190477 <https://reviews.llvm.org/D115661#3190477>, @yaxunl wrote:
>>>
>>>> This may cause perf regressions for HIP.
>>>
>>> Do you have a test that would show such a regression? Emitting a store to address space (4) in a constructor seems the wrong thing to do.
>>
>> The two lit tests which changed from addr space 4 to 1 demonstrated that. In alias analysis, if a variable is in addr space 4, the backend knows that it is constant and can do optimizations on it. After changing to addr space 1, those optimizations are gone.
>
> The backend also knows because the constant flag is set on the global variable. Addrspace(4) is a kludge which is largely redundant with other mechanisms for indicating constants

If backend can only rely on constant flag then we do not need put global variables in constant addr space.

Let's leave this patch as it is now. And revisit it if there are any regressions found.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115661



More information about the cfe-commits mailing list