[PATCH] D61711: [ELF][Driver] Fix precedence of symbol ordering file and CGProfile

[Tiancong Wang via Phabricator via llvm-commits]

tcwang updated this revision to Diff 198874.
tcwang added a comment.

This patch is a fix for https://bugs.llvm.org/show_bug.cgi?id=41804.
We try to solve the precedence of user-specified symbol ordering file and C3 ordering provided as call graph. It deals with two case:
(1) When both --symbol-ordering-file=<file> and --call-graph-order-file=<file> are present, LLD reports an error and aborts.
(2) When only --symbol-ordering-file=<file> is present, it takes precedence over implicit call graph (CGProfile) generated by CGProfilePass enabled in new pass manager.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61711/new/

https://reviews.llvm.org/D61711

Files:
  lld/ELF/Driver.cpp
  lld/test/ELF/symbol-ordering-file-cgprofile-conflicts.s


Index: lld/test/ELF/symbol-ordering-file-cgprofile-conflicts.s
===================================================================
--- /dev/null
+++ lld/test/ELF/symbol-ordering-file-cgprofile-conflicts.s
@@ -0,0 +1,67 @@
+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1
+// RUN: ld.lld -e A %t1 --no-call-graph-profile-sort -o %t2
+// RUN: llvm-nm --numeric-sort %t2 | FileCheck %s --check-prefix=NO_ORDERING
+// NO_ORDERING:         0000000000201000 t D
+// NO_ORDERING-NEXT:    0000000000201001 T C
+// NO_ORDERING-NEXT:    0000000000201002 T B
+// NO_ORDERING-NEXT:    0000000000201003 T A	
+
+// RUN: ld.lld -e A %t1 -o %t2
+// RUN: llvm-nm --numeric-sort %t2 | FileCheck %s --check-prefix=CALL_GRAPH
+// CALL_GRAPH:		0000000000201000 T A
+// CALL_GRAPH-NEXT:	0000000000201000 t Aa
+// CALL_GRAPH-NEXT:	0000000000201001 T B
+// CALL_GRAPH-NEXT:	0000000000201002 T C
+// CALL_GRAPH-NEXT:	0000000000201003 t D
+
+// RUN: rm %t.symbol_order || true
+// RUN: echo "C" >> %t.symbol_order
+// RUN: echo "B" >> %t.symbol_order
+// RUN: echo "D" >> %t.symbol_order
+// RUN: echo "A" >> %t.symbol_order
+
+// RUN: ld.lld -e A %t1 --symbol-ordering-file %t.symbol_order -o %t2
+// RUN: llvm-nm --numeric-sort %t2 | FileCheck %s --check-prefix=SYMBOL_ORDER
+// SYMBOL_ORDER:	0000000000201000 T C
+// SYMBOL_ORDER-NEXT:	0000000000201001 T B
+// SYMBOL_ORDER-NEXT:	0000000000201002 t D
+// SYMBOL_ORDER-NEXT:	0000000000201003 T A
+
+// RUN: rm %t.call_graph || true
+// RUN: echo "A B 5" > %t.call_graph
+// RUN: echo "B C 50" >> %t.call_graph
+// RUN: echo "C D 40" >> %t.call_graph
+// RUN: echo "D B 10" >> %t.call_graph
+
+// RUN: not ld.lld -e A %t1 --symbol-ordering-file %t.symbol_order --call-graph-ordering-file %t.call_graph -o %t2 2>&1 | FileCheck %s
+// RUN: not ld.lld -e A %t1 --call-graph-ordering-file %t.call_graph --symbol-ordering-file %t.symbol_order -o %t2 2>&1 | FileCheck %s
+// CHECK: error: --symbol-ordering-file should NOT be used together with --call-graph-order-file
+
+    .section    .text.D,"ax", at progbits
+D:
+    retq
+
+    .section    .text.C,"ax", at progbits
+    .globl  C
+C:
+    retq
+
+    .section    .text.B,"ax", at progbits
+    .globl  B
+B:
+    retq
+
+    .section    .text.A,"ax", at progbits
+    .globl  A
+A:
+Aa:
+    retq
+
+    .cg_profile A, B, 10
+    .cg_profile A, B, 10
+    .cg_profile Aa, B, 80
+    .cg_profile A, C, 40
+    .cg_profile B, C, 30
+    .cg_profile C, D, 90
+
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -964,9 +964,17 @@
   std::tie(Config->AndroidPackDynRelocs, Config->RelrPackDynRelocs) =
       getPackDynRelocs(Args);
 
-  if (auto *Arg = Args.getLastArg(OPT_symbol_ordering_file))
-    if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
+  if (auto *Arg = Args.getLastArg(OPT_symbol_ordering_file)){
+    if (Args.hasArg(OPT_call_graph_ordering_file))
+      error("--symbol-ordering-file should NOT be used together"
+            " with --call-graph-order-file");
+    if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue())){
       Config->SymbolOrderingFile = getSymbolOrderingFile(*Buffer);
+      // Also need to disable CallGraphProfileSort to prevent
+      // LLD order symbols with CGProfile
+      Config->CallGraphProfileSort = false;
+    }
+  }
 
   // If --retain-symbol-file is used, we'll keep only the symbols listed in
   // the file and discard all others.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61711.198874.patch
Type: text/x-patch
Size: 3509 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190509/09f54260/attachment.bin>