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

Tue Dec 14 13:53:17 PST 2021

estewart08 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 I am understanding you correctly, putting things in address space (4) has little to no performance benefit. @yaxunl seems to think otherwise. I agree that we can further constrain the address space (1) criteria, but I am getting conflicting viewpoints here on performance.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115661