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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
