[PATCH] D11568: fix memcpy/memset/memmove lowering when optimizing for size

[Sanjay Patel]

spatel added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4156
@@ +4155,3 @@
+  const Function *F = MF.getFunction();
+  bool hasMinSize = F->hasFnAttribute(Attribute::MinSize);
+  bool hasOptSize = F->hasFnAttribute(Attribute::OptimizeForSize);
----------------
mkuper wrote:
> Should start with a capital, I think.
Yep, will fix.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4159
@@ +4158,3 @@
+
+  // On Darwin, -Os means optimize for size without hurting performance, so
+  // only really optimize for size when -Oz (MinSize) is used.
----------------
mkuper wrote:
> Is this Darwin/non-Darwin distinction documented anywhere?
> And do we respect it anywhere else, aside from the memfunc issue?
AFAIK, it's not documented anywhere publicly. It goes back to the PowerPC days at Apple (so a bit strange that it's not also in the PPC lowering). 

At the time, Apple built almost all internal projects with -Os and made that the default opt level for developers too (not sure if this is still true today). 

The mem* lowering was an exception to the general rule to always optimize for size because it was clearly a win on some benchmark to inline those. So I assume this little hack was added to gcc and made it's way to LLVM too. :)

The existing test case references: rdar://8821501 Perhaps someone at Apple can give us an update. It may be that we don't even need to make this distinction for Darwin anymore? 

Ie, on x86 an inline 'rep movs' might be a better answer, and the AArch64 backend does not currently make any concessions for size on any platform:
   MaxStoresPerMemset = MaxStoresPerMemsetOptSize = 8;
   MaxStoresPerMemcpy = MaxStoresPerMemcpyOptSize = 4;
   MaxStoresPerMemmove = MaxStoresPerMemmoveOptSize = 4;





http://reviews.llvm.org/D11568