[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land. Thanks! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125974/new/ https://reviews.llvm.org/D125974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
MichielDerhaeg updated this revision to Diff 431768.
MichielDerhaeg marked an inline comment as done.
MichielDerhaeg added a comment.
- add target requirement to lit test
- move condition
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125974/new/
https://reviews.llvm.org/D125974
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/embed-bitcode-elf.c
Index: clang/test/Driver/embed-bitcode-elf.c
===
--- /dev/null
+++ clang/test/Driver/embed-bitcode-elf.c
@@ -0,0 +1,16 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang -c -target aarch64-linux-android21 %s -fembed-bitcode -o %t.o \
+// RUN: -ffunction-sections -fdata-sections -fstack-protector-strong
+
+// RUN: llvm-readobj -S %t.o | FileCheck --check-prefix=CHECK %s
+// CHECK: Name: .text.foo
+// CHECK: Name: .llvmbc
+// CHECK: Name: .llvmcmd
+
+// RUN: llvm-objcopy %t.o --dump-section=.llvmcmd=%t.llvmcmd
+// RUN: FileCheck --check-prefix=CMD %s < %t.llvmcmd
+// CMD: ffunction-sections
+// CMD: fdata-sections
+
+int foo() { return 42; }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4757,54 +4757,92 @@
// Render target options.
TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
-// reject options that shouldn't be supported in bitcode
-// also reject kernel/kext
-static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
-options::OPT_mkernel,
-options::OPT_fapple_kext,
-options::OPT_ffunction_sections,
-options::OPT_fno_function_sections,
-options::OPT_fdata_sections,
-options::OPT_fno_data_sections,
-options::OPT_fbasic_block_sections_EQ,
-options::OPT_funique_internal_linkage_names,
-options::OPT_fno_unique_internal_linkage_names,
-options::OPT_funique_section_names,
-options::OPT_fno_unique_section_names,
-options::OPT_funique_basic_block_section_names,
-options::OPT_fno_unique_basic_block_section_names,
-options::OPT_mrestrict_it,
-options::OPT_mno_restrict_it,
-options::OPT_mstackrealign,
-options::OPT_mno_stackrealign,
-options::OPT_mstack_alignment,
-options::OPT_mcmodel_EQ,
-options::OPT_mlong_calls,
-options::OPT_mno_long_calls,
-options::OPT_ggnu_pubnames,
-options::OPT_gdwarf_aranges,
-options::OPT_fdebug_types_section,
-options::OPT_fno_debug_types_section,
-options::OPT_fdwarf_directory_asm,
-options::OPT_fno_dwarf_directory_asm,
-options::OPT_mrelax_all,
-options::OPT_mno_relax_all,
-options::OPT_ftrap_function_EQ,
-options::OPT_ffixed_r9,
-options::OPT_mfix_cortex_a53_835769,
-options::OPT_mno_fix_cortex_a53_835769,
-options::OPT_ffixed_x18,
-options::OPT_mglobal_merge,
-options::OPT_mno_global_merge,
-options::OPT_mred_zone,
-options::OPT_mno_red_zone,
-options::OPT_Wa_COMMA,
-options::OPT_Xassembler,
-options::OPT_mllvm,
-};
-for (const auto : Args)
- if (llvm::is_contained(kBitcodeOptionIgnorelist, A->getOption().getID()))
-D.Diag(diag::err_drv_unsupported_embed_bitcode) << A->getSpelling();
+if (RawTriple.isOSDarwin())
+ for (const auto : Args) {
+// For Apple platforms, the following options are disallowed to limit
+// the ability to change the backend behaviour and ABI. This is to
+// ensure the bitcode can be retargeted to certain architectures.
+static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
+options::OPT_mkernel,
+options::OPT_fapple_kext,
+options::OPT_ffunction_sections,
+options::OPT_fno_function_sections,
+options::OPT_fdata_sections,
+options::OPT_fno_data_sections,
+options::OPT_fbasic_block_sections_EQ,
+options::OPT_funique_internal_linkage_names,
+options::OPT_fno_unique_internal_linkage_names,
+options::OPT_funique_section_names,
+options::OPT_fno_unique_section_names,
+options::OPT_funique_basic_block_section_names,
+options::OPT_fno_unique_basic_block_section_names,
+options::OPT_mrestrict_it,
+options::OPT_mno_restrict_it,
+options::OPT_mstackrealign,
+options::OPT_mno_stackrealign,
+options::OPT_mstack_alignment,
+options::OPT_mcmodel_EQ,
+options::OPT_mlong_calls,
+options::OPT_mno_long_calls,
+options::OPT_ggnu_pubnames,
+options::OPT_gdwarf_aranges,
+options::OPT_fdebug_types_section,
+
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
wanders added a comment. This looks sensible to me. But I'm not an expert in the area (and still not 100% clear on the purpose of the ignorelist) so can't really say if the added comments captures the intent accurately. Comment at: clang/test/Driver/embed-bitcode-elf.c:1 +// RUN: %clang -c -target aarch64-linux-android21 %s -fembed-bitcode -o %t.o \ +// RUN: -ffunction-sections -fdata-sections -fstack-protector-strong Should this test have a `// REQUIRES: aarch64-registered-target`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125974/new/ https://reviews.llvm.org/D125974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
MichielDerhaeg marked an inline comment as done.
MichielDerhaeg added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4760-4762
// reject options that shouldn't be supported in bitcode
// also reject kernel/kext
static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
wanders wrote:
> This comment and variable name are not as accurate any longer. Maybe moving
> it inside `if (RawTriple.isOSDarwin())` makes it clearer.
Changed the comment to what I could derive from {D61627}.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125974/new/
https://reviews.llvm.org/D125974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
MichielDerhaeg updated this revision to Diff 431479.
MichielDerhaeg added a comment.
Herald added a reviewer: alexander-shaposhnikov.
- Render options for other platforms and test
- comment and propagate list
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125974/new/
https://reviews.llvm.org/D125974
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/embed-bitcode-elf.c
Index: clang/test/Driver/embed-bitcode-elf.c
===
--- /dev/null
+++ clang/test/Driver/embed-bitcode-elf.c
@@ -0,0 +1,13 @@
+// RUN: %clang -c -target aarch64-linux-android21 %s -fembed-bitcode -o %t.o \
+// RUN: -ffunction-sections -fdata-sections -fstack-protector-strong
+// RUN: llvm-readobj -S %t.o | FileCheck --check-prefix=CHECK %s
+// CHECK: Name: .text.foo
+// CHECK: Name: .llvmbc
+// CHECK: Name: .llvmcmd
+//
+// RUN: llvm-objcopy %t.o --dump-section=.llvmcmd=%t.llvmcmd
+// RUN: FileCheck --check-prefix=CMD %s < %t.llvmcmd
+// CMD: ffunction-sections
+// CMD: fdata-sections
+
+int foo() { return 42; }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4757,54 +4757,91 @@
// Render target options.
TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
-// reject options that shouldn't be supported in bitcode
-// also reject kernel/kext
-static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
-options::OPT_mkernel,
-options::OPT_fapple_kext,
-options::OPT_ffunction_sections,
-options::OPT_fno_function_sections,
-options::OPT_fdata_sections,
-options::OPT_fno_data_sections,
-options::OPT_fbasic_block_sections_EQ,
-options::OPT_funique_internal_linkage_names,
-options::OPT_fno_unique_internal_linkage_names,
-options::OPT_funique_section_names,
-options::OPT_fno_unique_section_names,
-options::OPT_funique_basic_block_section_names,
-options::OPT_fno_unique_basic_block_section_names,
-options::OPT_mrestrict_it,
-options::OPT_mno_restrict_it,
-options::OPT_mstackrealign,
-options::OPT_mno_stackrealign,
-options::OPT_mstack_alignment,
-options::OPT_mcmodel_EQ,
-options::OPT_mlong_calls,
-options::OPT_mno_long_calls,
-options::OPT_ggnu_pubnames,
-options::OPT_gdwarf_aranges,
-options::OPT_fdebug_types_section,
-options::OPT_fno_debug_types_section,
-options::OPT_fdwarf_directory_asm,
-options::OPT_fno_dwarf_directory_asm,
-options::OPT_mrelax_all,
-options::OPT_mno_relax_all,
-options::OPT_ftrap_function_EQ,
-options::OPT_ffixed_r9,
-options::OPT_mfix_cortex_a53_835769,
-options::OPT_mno_fix_cortex_a53_835769,
-options::OPT_ffixed_x18,
-options::OPT_mglobal_merge,
-options::OPT_mno_global_merge,
-options::OPT_mred_zone,
-options::OPT_mno_red_zone,
-options::OPT_Wa_COMMA,
-options::OPT_Xassembler,
-options::OPT_mllvm,
-};
-for (const auto : Args)
- if (llvm::is_contained(kBitcodeOptionIgnorelist, A->getOption().getID()))
+for (const auto : Args) {
+ // For Apple platforms, the following options are disallowed to limit the
+ // ability to change the backend behaviour and ABI. This is to ensure the
+ // bitcode can be retargeted to certain architectures.
+ static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
+ options::OPT_mkernel,
+ options::OPT_fapple_kext,
+ options::OPT_ffunction_sections,
+ options::OPT_fno_function_sections,
+ options::OPT_fdata_sections,
+ options::OPT_fno_data_sections,
+ options::OPT_fbasic_block_sections_EQ,
+ options::OPT_funique_internal_linkage_names,
+ options::OPT_fno_unique_internal_linkage_names,
+ options::OPT_funique_section_names,
+ options::OPT_fno_unique_section_names,
+ options::OPT_funique_basic_block_section_names,
+ options::OPT_fno_unique_basic_block_section_names,
+ options::OPT_mrestrict_it,
+ options::OPT_mno_restrict_it,
+ options::OPT_mstackrealign,
+ options::OPT_mno_stackrealign,
+ options::OPT_mstack_alignment,
+ options::OPT_mcmodel_EQ,
+ options::OPT_mlong_calls,
+ options::OPT_mno_long_calls,
+ options::OPT_ggnu_pubnames,
+ options::OPT_gdwarf_aranges,
+ options::OPT_fdebug_types_section,
+ options::OPT_fno_debug_types_section,
+ options::OPT_fdwarf_directory_asm,
+ options::OPT_fno_dwarf_directory_asm,
+ options::OPT_mrelax_all,
+
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
wanders added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4760-4762
// reject options that shouldn't be supported in bitcode
// also reject kernel/kext
static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
This comment and variable name are not as accurate any longer. Maybe moving it
inside `if (RawTriple.isOSDarwin())` makes it clearer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125974/new/
https://reviews.llvm.org/D125974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin
steven_wu added a comment. LGTM. Can you add a test for your usecase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125974/new/ https://reviews.llvm.org/D125974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
