jasonliu added inline comments.
================ Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596 } + // Include the filename in the symbol name. Including "sub_" matches gcc ---------------- jasonliu wrote: > jasonliu wrote: > > I think this patch is missing what @hubert.reinterpretcast mentioned in > > https://reviews.llvm.org/D74166?id=269900#inline-751064 > > which is an early return like this: > > > > ``` > > if (CXXGlobalInits.empty()) > > return; > > ``` > Please double check the above early return is desired though. It seems even > when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a > lot of things with `Fn` passed in. Later, we also called `AddGlobalCtor(Fn)`. > So a lot of behavior changes here, we want to make sure it's really 'NFC'. FYI, ``` class test { public: test(); }; test t1 __attribute__((init_priority (300))); ``` Compile with `clang -target x86_64-apple-darwin10 -emit-llvm ` With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and also that function will not get added into @llvm.global_ctors. I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in this case. But this change does not really qualify as NFC. If we need to do this change, a test case is necessary, and we better ask people who familiar with the platform to confirm it's a desired change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81972/new/ https://reviews.llvm.org/D81972 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
