Page MenuHomePhabricator

[NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

Authored by Xiangling_L on Jun 16 2020, 2:53 PM.



Tidy up some code of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()as the pre-work of [AIX][Frontend] Static init implementation for AIX considering no priority [] patch.

Diff Detail

Event Timeline

Xiangling_L created this revision.Jun 16 2020, 2:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2020, 2:53 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Xiangling_L edited the summary of this revision. (Show Details)Jun 16 2020, 2:55 PM
jasonliu added inline comments.Jun 16 2020, 3:20 PM

I think this patch is missing what @hubert.reinterpretcast mentioned in
which is an early return like this:

if (CXXGlobalInits.empty())
Xiangling_L marked an inline comment as done.

Minor change;

jasonliu added inline comments.Jun 17 2020, 9:27 AM

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'.

jasonliu added inline comments.Jun 17 2020, 1:27 PM


class 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.

Xiangling_L marked 3 inline comments as done.

Remove early return part;


Thanks, I am gonna remove this part and keep it as a NFC patch.

New diff does not have context available.

Add the context to the patch;

jasonliu accepted this revision.Jun 17 2020, 2:57 PM



Sure. Please update D74166 accordingly.

This revision is now accepted and ready to land.Jun 17 2020, 2:57 PM

In the less complex context of this patch, it seems this should just initialize FileName with getTransformedFileName(getModule()) by having getTransformedFileName return a SmallString<128> directly instead of a StringRef backed by a SmallString<128>. I think this would still translate well to D74166.

Xiangling_L marked 2 inline comments as done.

Address comments;

This revision was automatically updated to reflect the committed changes.