[PATCH] D40407: [COFF] Implement constructor priorities

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1225-1229
+  std::string Name;
+  if (IsCtor)
+    Name = ".ctors";
+  else
+    Name = ".dtors";
----------------
ruiu wrote:
> Maybe `std::string Name = IsCtor ? ".ctors" : ".dtors"` is slightly more readable?
Sure, I can change that.


================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1233
+    Number << '.' << std::setw(5) << std::setfill('0') << (65535 - Priority);
+    Name += Number.str();
+  }
----------------
Do you have any suggestion on a more idiomatic way of doing the equivalent of `snprintf("%05u", 65535 - Priority)` here? This feels quite roundabout.


================
Comment at: test/CodeGen/X86/constructor.ll:33
 ; CTOR-NEXT:	.quad	g
+; CTOR-NEXT:	.section	.ctors.9980,"aGw", at progbits,v,comdat
+; CTOR-NEXT:	.p2align	3
----------------
ruiu wrote:
> Is this correct? I thought it would be .ctors.09980.
I'm actually not sure. My GCC reference doesn't output .ctors on linux (and can't easily be made to output it either, it seems). GNU ld does handle it correctly with .ctors without the zero padding on linux, but not for mingw though. But I found this GCC bug report, which discussed similar matters, and it does seem like it did do zero padding there as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46770

So I guess it makes sense to make it consistent and add the zero padding there as well. If the linker does interpret it numerically, it will still work, and if it doesn't, zero padding is the only way it can work.


https://reviews.llvm.org/D40407