[PATCH] Add comdat key field to llvm.global_ctors and llvm.global_dtors

Reid Kleckner rnk at google.com
Thu May 8 16:14:12 PDT 2014


================
Comment at: include/llvm/Target/TargetLoweringObjectFile.h:142
@@ -141,1 +141,3 @@
+                       const MCSection *KeySec = 0) const {
+    (void)KeySym;
     return StaticDtorSection;
----------------
Saleem Abdulrasool wrote:
> 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?
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.

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1301
@@ +1300,3 @@
+struct Structor {
+  Structor() : Priority(0), Func(0), ComdatKey(0) {}
+  int Priority;
----------------
Saleem Abdulrasool wrote:
> Can you use nullptr or NULL for the pointers in the initializer list please.
done

================
Comment at: lib/IR/Verifier.cpp:413
@@ +412,3 @@
+      if (STy->getNumElements() == 3) {
+        Type *ETy = STy->getTypeAtIndex(2);
+        Assert1(ETy->isPointerTy() &&
----------------
Saleem Abdulrasool wrote:
> This is going to cause a warning on non-assert builds wont it?  Perhaps use a DEBUG_ONLY wrapper around this?
Assert1() is not compiled out in NDEBUG builds, it's utility for the verifier.  It wraps a return statement. =/

================
Comment at: lib/IR/Verifier.cpp:415
@@ +414,3 @@
+        Assert1(ETy->isPointerTy() &&
+                    cast<PointerType>(ETy)->getElementType()->isIntegerTy(8),
+                "wrong type for intrinsic global variable", &GV);
----------------
Saleem Abdulrasool wrote:
> Shouldnt this be aligned with ETy?
clang-format really doesn't want to put it there because then it would line up with the following argument after the comma.  *shrug*

http://reviews.llvm.org/D3499






More information about the llvm-commits mailing list