[PATCH] D16145: Stop increasing alignment of externally-visible globals on ELF platforms.

[Tim Northover via llvm-commits]

t.p.northover added a comment.

Code looks good to me too, and the comments despite me piling on with the nits.

I don't really care about the sentence, but we should probably be a little more conservative with the PIC in case someone actually comes along and does it without investigating properly.


================
Comment at: lib/IR/Globals.cpp:153-156
@@ +152,6 @@
+  // increase the alignment of any variable which might be emitted
+  // into a shared library, and which is exported. If the main
+  // executable accesses a variable found in a shared-lib, the main
+  // exe actually allocates memory for, and exports the symbol ITSELF,
+  // taking precendence over the symbol found in the library. At build
+  // time, the observed alignment of the variable is copied into the
----------------
Minor nit, but this isn't actually a sentence.

I'd also probably be more explicit about it being "link time" that the copying happens.

================
Comment at: lib/IR/Globals.cpp:170-173
@@ +169,6 @@
+  //
+  // TODO: is it possible to determine if the relocation mode is PIC
+  // at this point in the code? I think it'd be correct to skip this
+  // check in non-PIC mode. We'd need something like:
+  //  TargetMachine.getRelocationModel() == Reloc::PIC_;
+
----------------
It depends what we want to support, but non-PIC shared libraries certainly used to be a thing. They're now strongly discouraged, but we'd probably still have to think carefully before breaking them.


http://reviews.llvm.org/D16145