Change the cap on the amount of padding for each vtable to 32-byte (previously it was 128-byte)

[zhaomo]

Summary

We tested different cap values with a recent commit of Chromium. Our results show that the 32-byte cap yields the smallest binary and all the caps yield similar performance.
Based on the results, we propose to change the cap value to 32-byte.

[zhaomo]

• removed .arcconfig

[dberris]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

774–777 ↗

(On Diff #155782)

Is it possible to have a reference to the experiment results either through a link or a document or in a comment?

[zhaomo]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

774–777 ↗

(On Diff #155782)

How about a link to the email of the experiment results (http://lists.llvm.org/pipermail/llvm-dev/2018-July/124694.html)? Is the llvm-dev archive considered permanent?

[dberris]

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

774–777 ↗

(On Diff #155782)

If it could be summarised somewhere in this file, it would be more permanent and less cryptic than the current comment. I can suggest a summary of the methodology, for example, even citing the trade-off being done. Something like:

Experiments with some large applications with a non-trivial number of dynamic types
have shown a binary size reduction of N% with virtually no measurable runtime overhead.
This padding used to be larger (128) and based on more recent experiments (see
<link to email or something in the LLVM docs/ directory>) we found that it could be
reduced to 32 and get significant binary size reductions with comparable runtime
performance.

Alternatively, if we can make the number a function of the cache line size (say, instead of 32 use the target's cache line size and derive something like 32 from it) which might/could explain why aligning to these numbers on those boundaries would not cause significant slowdowns but still have good binary size effects/properties. If I read the experiment results I can see that the platforms being built for/tested are 64-bit platforms with (I think) 64-byte cache lines.

[zhaomo]

Added more information about the experiments in the comment.

[pcc]

This looks fine if Dean is happy.

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

775 ↗

(On Diff #156185)

Remove the word "now".

[dberris]

LGTM -- +1 to @pcc's suggestion on wording of the comment.

[zhaomo]

Updated the comment according to Peter's suggestion.

[pcc]

It looks like this patch causes the check-cfi tests to fail, can you please take a look?

[zhaomo]

Some tests failed because they did not expect the new layouts caused by the change to the padding cap. I modified those tests so that now they accept the new layouts.

compiler-rt/trunk/test/cfi/create-derivers.test

 REQUIRES: asserts
 
 %% Explicit -flto to override possible -flto=thin in %clangxx_cfi
 RUN: %clangxx_cfi -flto -c -o %t1.o %S/simple-fail.cpp
 RUN: opt -lowertypetests -debug-only=lowertypetests -o /dev/null %t1.o 2>&1 | FileCheck --check-prefix=B0 %s
 B0: {{1B|B@@}}: {{.*}} size 1
 
 RUN: %clangxx_cfi -DB32 -flto -c -o %t2.o %S/simple-fail.cpp
 RUN: opt -lowertypetests -debug-only=lowertypetests -o /dev/null %t2.o 2>&1 | FileCheck --check-prefix=B32 %s
-B32: {{1B|B@@}}: {{.*}} size 24
+B32: {{1B|B@@}}: {{.*}} size 2{{3|4}}
 B32-NOT: all-ones
 
 RUN: %clangxx_cfi -DB64 -flto -c -o %t3.o %S/simple-fail.cpp
 RUN: opt -lowertypetests -debug-only=lowertypetests -o /dev/null %t3.o 2>&1 | FileCheck --check-prefix=B64 %s
-B64: {{1B|B@@}}: {{.*}} size 54
+B64: {{1B|B@@}}: {{.*}} size 5{{3|4}}
 B64-NOT: all-ones
 
 RUN: %clangxx_cfi -DBM -flto -c -o %t4.o %S/simple-fail.cpp
 RUN: opt -lowertypetests -debug-only=lowertypetests -o /dev/null %t4.o 2>&1 | FileCheck --check-prefix=BM %s
-BM: {{1B|B@@}}: {{.*}} size 84
+BM: {{1B|B@@}}: {{.*}} size 8{{3|4}}
 BM-NOT: all-ones

llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp

void LowerTypeTestsModule::buildBitSetsFromGlobalVariables(
   for (GlobalTypeMember *G : Globals) {
     GlobalVariable *GV = cast<GlobalVariable>(G->getGlobal());
     GlobalInits.push_back(GV->getInitializer());
     uint64_t InitSize = DL.getTypeAllocSize(GV->getValueType());
 
     // Compute the amount of padding required.
     uint64_t Padding = NextPowerOf2(InitSize - 1) - InitSize;
 
-    // Cap at 128 was found experimentally to have a good data/instruction
-    // overhead tradeoff.
-    if (Padding > 128)
-      Padding = alignTo(InitSize, 128) - InitSize;
+    // Experiments of different caps with Chromium on both x64 and ARM64
+    // have shown that the 32-byte cap generates the smallest binary on
+    // both platforms while different caps yield similar performance.
+    // (see https://lists.llvm.org/pipermail/llvm-dev/2018-July/124694.html)
+    if (Padding > 32)
+      Padding = alignTo(InitSize, 32) - InitSize;
 
     GlobalInits.push_back(
         ConstantAggregateZero::get(ArrayType::get(Int8Ty, Padding)));
   }
   if (!GlobalInits.empty())
     GlobalInits.pop_back();
   Constant *NewInit = ConstantStruct::getAnon(M.getContext(), GlobalInits);
   auto *CombinedGlobal =