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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
