quinnp updated this revision to Diff 446597.
quinnp marked 3 inline comments as done.
quinnp added a comment.

Adressed review comments.

- Modified how `llvm-lto` test-cases check the `llvm-objdump -t` output.
- Renamed `gold-lto-sections.c` to `forwarding-sections-liblto.c` and modified 
the test to use the `RUN` lines from `forwarding-sections-liblto.c` with the 
target specified using `--target=`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/forwarding-sections-liblto.c
  clang/test/Driver/gold-lto-sections.c
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/test/LTO/PowerPC/data-sections-aix.ll
  llvm/test/LTO/PowerPC/data-sections-linux.ll

Index: llvm/test/LTO/PowerPC/data-sections-linux.ll
===================================================================
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-linux.ll
@@ -0,0 +1,21 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: llvm-objdump -t %t/default.o | FileCheck %s
+; RUN: llvm-objdump -t %t/data-sections.o | FileCheck %s
+; RUN: llvm-objdump -t %t/no-data-sections.o | FileCheck --check-prefix \
+; RUN:   CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@var = global i32 0
+
+; CHECK:                      0000000000000000 g O .bss.var {{.*}} var
+
+; CHECK-NO-DATA-SECTIONS-NOT: .var
+; CHECK-NO-DATA-SECTIONS:     0000000000000000 g O .bss {{.*}} var
Index: llvm/test/LTO/PowerPC/data-sections-aix.ll
===================================================================
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-aix.ll
@@ -0,0 +1,21 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: llvm-objdump -t %t/default.o | FileCheck %s
+; RUN: llvm-objdump -t %t/data-sections.o | FileCheck %s
+; RUN: llvm-objdump -t %t/no-data-sections.o | FileCheck --check-prefix \
+; RUN:   CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc-ibm-aix7.2.0.0"
+
+@var = global i32 0
+
+; CHECK-NOT:              (csect: .data)
+; CHECK:                  00000000 g O .data {{.*}} var
+
+; CHECK-NO-DATA-SECTIONS: 00000000 g O .data (csect: .data) {{.*}} var
Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===================================================================
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/ParallelCG.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/Config/config.h"
@@ -344,6 +345,11 @@
       Config.CPU = "cyclone";
   }
 
+  // If data-sections is not explicitly set or unset, set data-sections by
+  // default to match the behaviour of lld and gold plugin.
+  if (!codegen::getExplicitDataSections())
+    Config.Options.DataSections = true;
+
   TargetMach = createTargetMachine();
   assert(TargetMach && "Unable to create target machine");
 
Index: clang/test/Driver/gold-lto-sections.c
===================================================================
--- clang/test/Driver/gold-lto-sections.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// RUN: touch %t.o
-//
-// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
-// RUN:     -Wl,-plugin-opt=foo -O3 \
-// RUN:     -ffunction-sections -fdata-sections \
-// RUN:     | FileCheck %s
-// CHECK: "-plugin-opt=-function-sections"
-// CHECK: "-plugin-opt=-data-sections"
Index: clang/test/Driver/forwarding-sections-liblto.c
===================================================================
--- /dev/null
+++ clang/test/Driver/forwarding-sections-liblto.c
@@ -0,0 +1,17 @@
+// RUN: touch %t.o
+// RUN: %clang --target=x86_64-unknown-linux -### %t.o -flto 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN:   -ffunction-sections -fdata-sections | FileCheck %s \
+// RUN:   --check-prefix=CHECK-SECTIONS
+// RUN: %clang --target=x86_64-unknown-linux -### %t.o -flto 2>&1 \
+// RUN:   -fno-function-sections -fno-data-sections | FileCheck %s \
+// RUN:   --check-prefix=CHECK-NO-SECTIONS
+
+// CHECK-NOT: "-plugin-opt=-function-sections=1"
+// CHECK-NOT: "-plugin-opt=-function-sections=0"
+// CHECK-NOT: "-plugin-opt=-data-sections=1"
+// CHECK-NOT: "-plugin-opt=-data-sections=0"
+// CHECK-SECTIONS: "-plugin-opt=-function-sections=1"
+// CHECK-SECTIONS: "-plugin-opt=-data-sections=1"
+// CHECK-NO-SECTIONS: "-plugin-opt=-function-sections=0"
+// CHECK-NO-SECTIONS: "-plugin-opt=-data-sections=0"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -567,14 +567,16 @@
       isUseSeparateSections(ToolChain.getEffectiveTriple());
 
   if (Args.hasFlag(options::OPT_ffunction_sections,
-                   options::OPT_fno_function_sections, UseSeparateSections)) {
-    CmdArgs.push_back("-plugin-opt=-function-sections");
-  }
+                   options::OPT_fno_function_sections, UseSeparateSections))
+    CmdArgs.push_back("-plugin-opt=-function-sections=1");
+  else if (Args.hasArg(options::OPT_fno_function_sections))
+    CmdArgs.push_back("-plugin-opt=-function-sections=0");
 
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
-                   UseSeparateSections)) {
-    CmdArgs.push_back("-plugin-opt=-data-sections");
-  }
+                   UseSeparateSections))
+    CmdArgs.push_back("-plugin-opt=-data-sections=1");
+  else if (Args.hasArg(options::OPT_fno_data_sections))
+    CmdArgs.push_back("-plugin-opt=-data-sections=0");
 
   // Pass an option to enable split machine functions.
   if (auto *A = Args.getLastArg(options::OPT_fsplit_machine_functions,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to