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