MaskRay updated this revision to Diff 319174.
MaskRay marked 8 inline comments as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85474/new/
https://reviews.llvm.org/D85474
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/fbinutils-version.c
llvm/include/llvm/MC/MCAsmInfo.h
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/LLVMTargetMachine.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/TargetMachine.cpp
llvm/test/CodeGen/X86/explicit-section-mergeable.ll
llvm/tools/llc/llc.cpp
Index: llvm/tools/llc/llc.cpp
===================================================================
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -82,6 +82,15 @@
cl::value_desc("N"),
cl::desc("Repeat compilation N times for timing"));
+static cl::opt<std::string>
+ BinutilsVersion("binutils-version", cl::Hidden,
+ cl::desc("Produced object files can use all ELF features "
+ "supported by this binutils version and newer."
+ "If -no-integrated-as is specified, the generated "
+ "assembly will consider GNU as support."
+ "'none' means that all ELF features can be used, "
+ "regardless of binutils support"));
+
static cl::opt<bool>
NoIntegratedAssembler("no-integrated-as", cl::Hidden,
cl::desc("Disable integrated assembler"));
@@ -430,6 +439,8 @@
TargetOptions Options;
auto InitializeOptions = [&](const Triple &TheTriple) {
Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);
+ Options.BinutilsVersion =
+ TargetMachine::parseBinutilsVersion(BinutilsVersion);
Options.DisableIntegratedAS = NoIntegratedAssembler;
Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
Index: llvm/test/CodeGen/X86/explicit-section-mergeable.ll
===================================================================
--- llvm/test/CodeGen/X86/explicit-section-mergeable.ll
+++ llvm/test/CodeGen/X86/explicit-section-mergeable.ll
@@ -282,15 +282,21 @@
;; --no-integrated-as avoids the use of ",unique," for compatibility with older binutils.
;; Error if an incompatible symbol is explicitly placed into a mergeable section.
-; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as 2>&1 \
+; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
; RUN: | FileCheck %s --check-prefix=NO-I-AS-ERR
; NO-I-AS-ERR: error: Symbol 'explicit_default_1' from module '<stdin>' required a section with entry-size=0 but was placed in section '.rodata.cst16' with entry-size=16: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
; NO-I-AS-ERR: error: Symbol 'explicit_default_4' from module '<stdin>' required a section with entry-size=0 but was placed in section '.debug_str' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
; NO-I-AS-ERR: error: Symbol 'explicit_implicit_2' from module '<stdin>' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
; NO-I-AS-ERR: error: Symbol 'explicit_implicit_4' from module '<stdin>' required a section with entry-size=0 but was placed in section '.rodata.str1.1' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
+;; For GNU as before 2.35,
;; Don't create mergeable sections for globals with an explicit section name.
; RUN: echo '@explicit = unnamed_addr constant [2 x i16] [i16 1, i16 1], section ".explicit"' > %t.no_i_as.ll
-; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as 2>&1 \
-; RUN: | FileCheck %s --check-prefix=NO-I-AS
-; NO-I-AS: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-OLD
+; NO-I-AS-OLD: .section .explicit,"a",@progbits
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=2.35 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as -binutils-version=none 2>&1 \
+; RUN: | FileCheck %s --check-prefix=NO-I-AS-NEW
+; NO-I-AS-NEW: .section .explicit,"aM",@progbits,4,unique,1
Index: llvm/lib/Target/TargetMachine.cpp
===================================================================
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -233,3 +233,12 @@
return TargetIRAnalysis(
[this](const Function &F) { return this->getTargetTransformInfo(F); });
}
+
+std::pair<int, int> TargetMachine::parseBinutilsVersion(StringRef Version) {
+ if (Version == "none")
+ return {INT_MAX, 0}; // Make binutilsIsAtLeast() return true.
+ std::pair<int, int> Ret;
+ if (!Version.consumeInteger(10, Ret.first) && Version.consume_front("."))
+ Version.consumeInteger(10, Ret.second);
+ return Ret;
+}
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===================================================================
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -685,7 +685,8 @@
UniqueID = NextUniqueID++;
Flags |= ELF::SHF_LINK_ORDER;
} else {
- if (getContext().getAsmInfo()->useIntegratedAssembler()) {
+ if (getContext().getAsmInfo()->useIntegratedAssembler() ||
+ getContext().getAsmInfo()->binutilsIsAtLeast(2, 35)) {
// Symbols must be placed into sections with compatible entry
// sizes. Generate unique sections for symbols that have not
// been assigned to compatible sections.
@@ -736,8 +737,9 @@
assert(Section->getLinkedToSymbol() == LinkedToSym &&
"Associated symbol mismatch between sections");
- if (!getContext().getAsmInfo()->useIntegratedAssembler()) {
- // If we are not using the integrated assembler then this symbol might have
+ if (!(getContext().getAsmInfo()->useIntegratedAssembler() ||
+ getContext().getAsmInfo()->binutilsIsAtLeast(2, 35))) {
+ // If we are using GNU as before 2.35, then this symbol might have
// been placed in an incompatible mergeable section. Emit an error if this
// is the case to avoid creating broken output.
if ((Section->getFlags() & ELF::SHF_MERGE) &&
Index: llvm/lib/CodeGen/LLVMTargetMachine.cpp
===================================================================
--- llvm/lib/CodeGen/LLVMTargetMachine.cpp
+++ llvm/lib/CodeGen/LLVMTargetMachine.cpp
@@ -61,6 +61,9 @@
"Make sure you include the correct TargetSelect.h"
"and that InitializeAllTargetMCs() is being invoked!");
+ if (Options.BinutilsVersion.first > 0)
+ TmpAsmInfo->setBinutilsVersion(Options.BinutilsVersion);
+
if (Options.DisableIntegratedAS)
TmpAsmInfo->setUseIntegratedAssembler(false);
Index: llvm/include/llvm/Target/TargetOptions.h
===================================================================
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -146,6 +146,10 @@
/// optimization should be disabled for the given machine function.
bool DisableFramePointerElim(const MachineFunction &MF) const;
+ /// If greater than 0, override the default value of
+ /// MCAsmInfo::BinutilsVersion.
+ std::pair<int, int> BinutilsVersion{0, 0};
+
/// UnsafeFPMath - This flag is enabled when the
/// -enable-unsafe-fp-math flag is specified on the command line. When
/// this flag is off (the default), the code generator is not allowed to
Index: llvm/include/llvm/Target/TargetMachine.h
===================================================================
--- llvm/include/llvm/Target/TargetMachine.h
+++ llvm/include/llvm/Target/TargetMachine.h
@@ -376,6 +376,8 @@
/// The integer bit size to use for SjLj based exception handling.
static constexpr unsigned DefaultSjLjDataSize = 32;
virtual unsigned getSjLjDataSize() const { return DefaultSjLjDataSize; }
+
+ static std::pair<int, int> parseBinutilsVersion(StringRef Version);
};
/// This class describes a target machine that is implemented with the LLVM
Index: llvm/include/llvm/MC/MCAsmInfo.h
===================================================================
--- llvm/include/llvm/MC/MCAsmInfo.h
+++ llvm/include/llvm/MC/MCAsmInfo.h
@@ -406,6 +406,11 @@
//===--- Integrated Assembler Information ----------------------------===//
+ // Generated object files can use all ELF features supported by GNU ld of
+ // this binutils version and later. INT_MAX means all features can be used,
+ // regardless of GNU ld support.
+ std::pair<int, int> BinutilsVersion = {2, 26};
+
/// Should we use the integrated assembler?
/// The integrated assembler should be enabled by default (by the
/// constructors) when failing to parse a valid piece of assembly (inline
@@ -673,9 +678,17 @@
return InitialFrameState;
}
+ void setBinutilsVersion(std::pair<int, int> Value) {
+ BinutilsVersion = Value;
+ }
+
/// Return true if assembly (inline or otherwise) should be parsed.
bool useIntegratedAssembler() const { return UseIntegratedAssembler; }
+ bool binutilsIsAtLeast(int Major, int Minor) const {
+ return BinutilsVersion >= std::make_pair(Major, Minor);
+ }
+
/// Set whether assembly (inline or otherwise) should be parsed.
virtual void setUseIntegratedAssembler(bool Value) {
UseIntegratedAssembler = Value;
Index: clang/test/Driver/fbinutils-version.c
===================================================================
--- /dev/null
+++ clang/test/Driver/fbinutils-version.c
@@ -0,0 +1,19 @@
+// RUN: %clang -### -c -target x86_64-linux %s -fbinutils-version=none 2>&1 | FileCheck %s --check-prefix=NONE
+
+// NONE: "-fbinutils-version=none"
+
+// RUN: %clang -### -c -target aarch64-linux %s -fbinutils-version=2.35 2>&1 | FileCheck %s
+
+// CHECK: "-fbinutils-version=2.35"
+
+// RUN: not %clang -c -target x86_64-linux %s -fbinutils-version=nan 2>&1 | FileCheck %s --check-prefix=ERR1
+
+// ERR1: error: invalid argument 'nan' to -fbinutils-version=
+
+// RUN: not %clang -c -target x86_64-linux %s -fbinutils-version=2. 2>&1 | FileCheck %s --check-prefix=ERR2
+
+// ERR2: error: invalid argument '2.' to -fbinutils-version=
+
+// RUN: not %clang -c -target x86_64-linux %s -fbinutils-version=3.-14 2>&1 | FileCheck %s --check-prefix=ERR3
+
+// ERR3: error: invalid argument '3.-14' to -fbinutils-version=
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1016,6 +1016,9 @@
Args.hasFlag(OPT_funroll_loops, OPT_fno_unroll_loops,
(Opts.OptimizationLevel > 1));
+ Opts.BinutilsVersion =
+ std::string(Args.getLastArgValue(OPT_fbinutils_version_EQ));
+
Opts.DebugNameTable = static_cast<unsigned>(
Args.hasArg(OPT_ggnu_pubnames)
? llvm::DICompileUnit::DebugNameTableKind::GNU
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4840,6 +4840,21 @@
IsIntegratedAssemblerDefault))
CmdArgs.push_back("-fno-verbose-asm");
+ // Parse 'none' or '$major.$minor'.
+ if (Arg *A = Args.getLastArg(options::OPT_fbinutils_version_EQ)) {
+ StringRef V = A->getValue();
+ unsigned Num;
+ if (V == "none")
+ A->render(Args, CmdArgs);
+ else if (!V.consumeInteger(10, Num) &&
+ (V.empty() || (V.consume_front(".") &&
+ !V.consumeInteger(10, Num) && V.empty())))
+ A->render(Args, CmdArgs);
+ else
+ D.Diag(diag::err_drv_invalid_argument_to_option)
+ << A->getValue() << A->getOption().getName();
+ }
+
if (!TC.useIntegratedAs())
CmdArgs.push_back("-no-integrated-as");
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -490,6 +490,8 @@
break;
}
+ Options.BinutilsVersion =
+ llvm::TargetMachine::parseBinutilsVersion(CodeGenOpts.BinutilsVersion);
Options.UseInitArray = CodeGenOpts.UseInitArray;
Options.DisableIntegratedAS = CodeGenOpts.DisableIntegratedAS;
Options.CompressDebugSections = CodeGenOpts.getCompressDebugSections();
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4016,6 +4016,11 @@
def fprofile_dir : Joined<["-"], "fprofile-dir=">, Group<f_Group>;
+def fbinutils_version_EQ : Joined<["-"], "fbinutils-version=">, Group<f_Group>,
+ HelpText<"Produced object files can use all ELF features supported by binutils "
+ "newer than the specified version. If -fno-integrated-as is specified, the "
+ "generated assembly will consider GNU as support. 'none' means that all ELF "
+ "features can be used, regardless of binutils support">;
def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, Group<f_Group>, Flags<[CoreOption, LinkOption]>;
def ld_path_EQ : Joined<["--"], "ld-path=">, Group<Link_Group>;
Index: clang/include/clang/Basic/CodeGenOptions.h
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -128,6 +128,12 @@
// "none": Disable sections/labels for basic blocks.
std::string BBSections;
+ // If set, override the default value of MCAsmInfo::BinutilsVersion. If
+ // DisableIntegratedAS is specified, the assembly output will consider GNU as
+ // support. "none" means that all ELF features can be used, regardless of
+ // binutils support.
+ std::string BinutilsVersion;
+
enum class FramePointerKind {
None, // Omit all frame pointers.
NonLeaf, // Keep non-leaf frame pointers.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -98,6 +98,10 @@
-Wl,--gc-sections on ELF platforms to the linking command, and possibly
adding -fdata-sections -ffunction-sections to the command generating
the shared object).
+- New option ``-fbinutils-version=`` specifies the targeted binutils version.
+ For example, ``-fbinutils-version=2.35`` means compatibility with GNU as/ld
+ before 2.35 is not needed: new features can be used and there is no need to
+ work around old GNU as/ld bugs.
Deprecated Compiler Flags
-------------------------
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits