This is an archive of the discontinued LLVM Phabricator instance.

Add comdat key field to llvm.global_ctors and llvm.global_dtors
ClosedPublic

Authored by rnk on Apr 24 2014, 5:53 PM.

Details

Summary

This allows us to put dynamic initializers for weak data into the same
comdat group as the data being initialized. This is necessary for MSVC
ABI compatibility. It should also allow GlobalOpt to fire more often
weak data on other platforms if we can sort out the initialization of
the guard variable.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk updated this revision to Diff 8825.Apr 24 2014, 5:53 PM
rnk retitled this revision from to Add comdat key field to llvm.global_ctors and llvm.global_dtors.
rnk updated this object.
rnk added reviewers: nicholas, nlewycky.
rnk added a subscriber: Unknown Object (MLST).
compnerd added inline comments.
include/llvm/Target/TargetLoweringObjectFile.h
142 ↗(On Diff #8825)

Why the cast to void to silence an unused variable warning for KeySym but not for KeySec nor Priority? You dont seem to do the same for the getStaticCtorSection. What is special about the KeySym parameter here?

lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1301 ↗(On Diff #8825)

Can you use nullptr or NULL for the pointers in the initializer list please.

lib/IR/Verifier.cpp
413 ↗(On Diff #8825)

This is going to cause a warning on non-assert builds wont it? Perhaps use a DEBUG_ONLY wrapper around this?

415 ↗(On Diff #8825)

Shouldnt this be aligned with ETy?

rnk added inline comments.May 8 2014, 4:14 PM
include/llvm/Target/TargetLoweringObjectFile.h
142 ↗(On Diff #8825)

I think parameters with default values used to trigger a certain gcc warning. I thought I nuked all the default parameters to fix this warning, though. Apparently not.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1301 ↗(On Diff #8825)

done

lib/IR/Verifier.cpp
413 ↗(On Diff #8825)

Assert1() is not compiled out in NDEBUG builds, it's utility for the verifier. It wraps a return statement. =/

415 ↗(On Diff #8825)

clang-format really doesn't want to put it there because then it would line up with the following argument after the comma. *shrug*

rnk updated this revision to Diff 9234.May 8 2014, 4:15 PM
  • Autoupgrade old IR and add an llvm-link test
rnk updated this revision to Diff 9494.May 16 2014, 12:40 PM
  • Add FIXMEs about removing the two-element form in LLVM 4.0
rnk accepted this revision.May 16 2014, 1:46 PM
rnk added a reviewer: rnk.

Nick approved this on IRC, committing.

This revision is now accepted and ready to land.May 16 2014, 1:46 PM
rnk closed this revision.May 19 2014, 7:24 AM
rnk updated this revision to Diff 9569.

Closed by commit rL209015 (authored by @rnk).