This is an archive of the discontinued LLVM Phabricator instance.

Start settinng dso_local for COFF
ClosedPublic

Authored by espindola on Feb 20 2018, 9:06 AM.

Details

Summary

With this there are still some GVs where we don't set dso_local because setGVProperties is never called. I intend to fix that in followup commits. This is just the bare minimum to teach shouldAssumeDSOLocal what it should do for COFF.

Diff Detail

Event Timeline

espindola created this revision.Feb 20 2018, 9:06 AM

Some inline comments - I think this looks ok in general, but I'm curious about the answers to the questions/documentation bits inline.

Thanks!

lib/CodeGen/CGDecl.cpp
257

If positioning is important here we should probably document it.

lib/CodeGen/CodeGenModule.cpp
721–729

This seems to need an update?

722

"firmware"

lib/CodeGen/ItaniumCXXABI.cpp
1650

Ditto with the moving comment from above.

Address review comments.

echristo added inline comments.Feb 22 2018, 4:22 PM
lib/CodeGen/CodeGenModule.h
728

Agreed? Maybe? Should the rest of the properties from above each call be set in this function?

Since you've got the decl it might be best to get as many of them in one place as possible?

echristo accepted this revision.Feb 22 2018, 4:49 PM
This revision is now accepted and ready to land.Feb 22 2018, 4:49 PM