[PATCH] D51377: [NFC] Make getPreferredAlignment honor section markings.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: lib/IR/DataLayout.cpp:834
+  // FIXME: Why 16, specifically?
   if (GV->hasInitializer() && GVAlignment == 0) {
     if (Alignment < 16) {
----------------
efriedma wrote:
> dmgreen wrote:
> > About this, it keeps adding very visible codesize (technially datasize) to our images, which because our linker reports all the padding is very visible to customers, and unnecissarily bloats the images.
> > 
> > I ran some benchmarks and couldn't find any places it was improving performance, at least on ARM/AArch64. Remove this actually improved things in a few places.
> > 
> > The over-aligning globals dates back to a long time ago, I'm just not sure it's worth doing on many architectures. From what I remember, removing it does fail some X86 tests though, with worse looking code.
> > 
> > We may need be smarter than that and come up with some way to remove it for targets (or a way to mark globals as optsize :-/ )
> Are you sure it's this code, specifically, causing the bloat?  As far as I know, clang explicitly marks the alignment of all globals it creates, which makes this code irrelevant.
Yes but...

The customer reported case I remember is the RTTI string are not aligned. Something like this:
```
class MyNormalClass {
    public:
            virtual int foo() const { return 1; };
};

class OveralignClass {
    public:
            virtual int foo() const { return 1; };
};

MyNormalClass a;
OveralignClass b;
```
"14OveralignClass", being 17 characters long, gets over aligned.

The other I found from looking around is the printf->puts conversion doesn't put and alignment on the strings it creates. I've just created D51410 for that one.

Anyway, none of that is really related to this patch. +1 from me, thanks for making this less easy to get wrong.


Repository:
  rL LLVM

https://reviews.llvm.org/D51377