[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 17:10:24 PDT 2022


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

There are some nits for tests. I'll step away and just LGTM.



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:579
+  else if (Args.hasArg(options::OPT_fno_data_sections))
+    CmdArgs.push_back("-plugin-opt=-data-sections=0");
 
----------------
Is -plugin-opt=-data-sections=0 a problem for `!UseSeparateSections` targets?


================
Comment at: clang/test/Driver/forwarding-sections-liblto.c:1
+// RUN: touch %t.o
+// RUN: %clang --target=x86_64-unknown-linux -### %t.o -flto 2>&1 | FileCheck %s
----------------
You may just place these tests into function-sections.c. It seems fine to additionally test data-sections in the file.

(Even if a new test is needed, I'd avoid `-liblto` and use `-lto` instead`. But the extra file seems too specific. The test can well be placed in an existing file.)


================
Comment at: clang/test/Driver/forwarding-sections-liblto.c:2
+// 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 \
----------------
Instead of `touch %t.o`, just use `%s`


================
Comment at: clang/test/Driver/forwarding-sections-liblto.c:10
+
+// CHECK-NOT: "-plugin-opt=-function-sections=1"
+// CHECK-NOT: "-plugin-opt=-function-sections=0"
----------------
Just use `// CHECK-NOT: "-plugin-opt=-function-sections`. No need to duplicate it for 0 and 1.

Ditto for data-sections


================
Comment at: llvm/test/LTO/PowerPC/data-sections-linux.ll:20
+
+; CHECK-NO-DATA-SECTIONS-NOT: .var
+; CHECK-NO-DATA-SECTIONS:     0000000000000000 g O .bss {{.*}} var
----------------
What does this `...-NOT: .var` do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401



More information about the cfe-commits mailing list