yaxunl added a comment. 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. I am not objecting to putting variables like `const float x=log2(y)` to addr space 1. However, the lit tests indicate the condition check is too restrictive. ================ Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:10 // X86: @_ZN1A3FooE ={{.*}} constant i32 123, align 4 -// AMD: @_ZN1A3FooE ={{.*}} addrspace(4) constant i32 123, align 4 +// AMD: @_ZN1A3FooE ={{.*}} addrspace(1) constant i32 123, align 4 const int *p = &A::Foo; // emit available_externally ---------------- estewart08 wrote: > yaxunl wrote: > > Do you know why this is not treated as constant initialization? > > > There is no initialization here: > ``` > const int A::Foo; > ``` > > It seems the compiler ignores the 123 in the struct. > > I see address space (4) when the test is written like this: > ``` > struct A { > static const int Foo; > }; > const int A::Foo = 123; > ``` In this case, there are two Decl's for `A::Foo` and the initializer of `A::Foo` is on the first initializer whereas `hasConstantInitialization` only checks the final Decl, which does not have an initializer. Therefore hasConstantInitialization may conservatively return false for a variable with a constant initializer in a previous decl. clang codegen calls VarDecl::getAnyInitializer to checks all Decls of a variable when emitting its initializer in LLVM IR https://github.com/llvm/llvm-project/blob/5336befe8c3cde08cec020583700b4d2ba25ac16/clang/lib/AST/Decl.cpp#L2277 I would suggest calling getAnyInitializer to get the initializer, then check whether it is constant expr to determine whether the variable will have a constant initializer in IR. I think hasConstantInitialization does not do that because it has multiple usages, not just for clang codegen, therefore it does not check all decls. 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
