[BOLT][AArch64] Ignore functions with constant islands during ICF and VeneerElimination

[treapster]

Summary

Don't fold functions with CIs, don't consider them veneers

[yota9]

bolt/include/bolt/Core/BinaryFunction.h

2016

What problem is solved here?

bolt/test/AArch64/prevent-ci-folding.s

3

[treapster]

bolt/include/bolt/Core/BinaryFunction.h

2016

The problem was that previous implementation returned true when there was a single $x symbol at the beginning of a function. Now we only consider data or dependencies.

[treapster]

Fix test

[yota9]

bolt/test/AArch64/prevent-ci-folding.s

5

Also build object using llvm-mc first

1. RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o

[treapster]

Fix test once more

[treapster]

Use llvm-mc

[yota9]

lgtm

bolt/include/bolt/Core/BinaryFunction.h

if (!OnBehalfOf) {
         Size = alignTo(Size, ExternalFunc->getConstantIslandAlignment());
         Size += ExternalFunc->estimateConstantIslandSize(this);
       }
     }
 
     return Size;
   }
 
-  bool hasIslandsInfo() const { return !!Islands; }
+  bool hasIslandsInfo() const {
+    return Islands && (hasConstantIsland() || !Islands->Dependency.empty());
+  }
 
   bool hasConstantIsland() const {
     return Islands && !Islands->DataOffsets.empty();
   }
 
   /// Return true iff the symbol could be seen inside this function otherwise
   /// it is probably another function.
   bool isSymbolValidInScope(const SymbolRef &Symbol, uint64_t SymbolSize) const;

bolt/lib/Core/BinaryFunction.cpp

void BinaryFunction::printLoopInfo(raw_ostream &OS) const {
   }
 
   OS << "Total number of loops: " << BLI->TotalLoops << "\n";
   OS << "Number of outer loops: " << BLI->OuterLoops << "\n";
   OS << "Maximum nested loop depth: " << BLI->MaximumDepth << "\n\n";
 }
 
 bool BinaryFunction::isAArch64Veneer() const {
-  if (empty())
+  if (empty() || hasIslandsInfo())
     return false;
 
   BinaryBasicBlock &BB = **BasicBlocks.begin();
   for (MCInst &Inst : BB)
     if (!BC.MIB->hasAnnotation(Inst, "AArch64Veneer"))
       return false;
 
   for (auto I = BasicBlocks.begin() + 1, E = BasicBlocks.end(); I != E; ++I) {

bolt/lib/Passes/IdenticalCodeFolding.cpp

bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
 
   if (A.getLayout().block_size() != B.getLayout().block_size())
     return false;
 
   // Comparing multi-entry functions could be non-trivial.
   if (A.isMultiEntry() || B.isMultiEntry())
     return false;
 
+  if (A.hasIslandsInfo() || B.hasIslandsInfo())
+    return false;
+
   // Process both functions in either DFS or existing order.
   const BinaryFunction::BasicBlockOrderType OrderA =
       opts::UseDFS
           ? A.dfs()
           : BinaryFunction::BasicBlockOrderType(A.getLayout().block_begin(),
                                                 A.getLayout().block_end());
   const BinaryFunction::BasicBlockOrderType OrderB =
       opts::UseDFS

bolt/test/AArch64/prevent-ci-folding.s

+// This test checks that functions containing Constant Islands are not folded even
+// if they have the same data
+
+// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+// RUN: %clang %cflags --target=aarch64-unknown-linux %t.o -o %t.exe -Wl,-q
+// RUN: llvm-bolt -icf -lite=false %t.exe -o %t.bolt
+// RUN: llvm-objdump -d -j .text %t.bolt | FileCheck %s
+
+// CHECK: <func1>:
+// CHECK: <func2>:
+
+func1:
+    add x0, x0, #1
+    ret
+    .word 0xdeadbeef
+    .word 0xdeadbeef
+.size func1, .-func1
+
+func2:
+    add x0, x0, #1
+    ret
+    .word 0xdeadbeef
+    .word 0xdeadbeef
+.size func2, .-func2
+
+.global        main
+.type  main, %function
+main:
+    mov x0, #0
+    bl     func1
+    bl     func2
+    sub     x0, x0, #2
+    mov     w8, #93
+    svc     #0