[MC] Remove Darwin SDK/Version from ObjFileInfo #103025
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's only a single user (MCMachOStreamer), so it makes more sense to move the version emission to the source of the data.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-llvm-binary-utilities Author: Alexis Engelke (aengelke) ChangesThere's only a single user (MCMachOStreamer), so it makes more sense to move the version emission to the source of the data. (Not sure whether we want this, but I don't see benefits in storing this information in the MCObjectFileInfo.) Full diff: https://github.com/llvm/llvm-project/pull/103025.diff 5 Files Affected:
diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index b661a43c88b08d..78019d39742e64 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -489,10 +489,6 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr<MCObjectFileInfo> MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
- if (Opts.DarwinTargetVariantTriple)
- MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
- if (!Opts.DarwinTargetVariantSDKVersion.empty())
- MOFI->setDarwinTargetVariantSDKVersion(Opts.DarwinTargetVariantSDKVersion);
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.GenDwarfForAssembly)
@@ -574,6 +570,13 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
Str.reset(TheTarget->createMCObjectStreamer(
T, Ctx, std::move(MAB), std::move(OW), std::move(CE), *STI));
Str.get()->initSections(Opts.NoExecStack, *STI);
+ if (T.isOSBinFormatMachO() && T.isOSDarwin()) {
+ Triple *TVT = Opts.DarwinTargetVariantTriple
+ ? &*Opts.DarwinTargetVariantTriple
+ : nullptr;
+ Str->emitVersionForTarget(T, VersionTuple(), TVT,
+ Opts.DarwinTargetVariantSDKVersion);
+ }
}
// When -fembed-bitcode is passed to clang_as, a 1-byte marker
diff --git a/llvm/include/llvm/MC/MCObjectFileInfo.h b/llvm/include/llvm/MC/MCObjectFileInfo.h
index dda3e8a020f3ae..e2a2c84e47910b 100644
--- a/llvm/include/llvm/MC/MCObjectFileInfo.h
+++ b/llvm/include/llvm/MC/MCObjectFileInfo.h
@@ -458,9 +458,6 @@ class MCObjectFileInfo {
private:
bool PositionIndependent = false;
MCContext *Ctx = nullptr;
- VersionTuple SDKVersion;
- std::optional<Triple> DarwinTargetVariantTriple;
- VersionTuple DarwinTargetVariantSDKVersion;
void initMachOMCObjectFileInfo(const Triple &T);
void initELFMCObjectFileInfo(const Triple &T, bool Large);
@@ -471,29 +468,6 @@ class MCObjectFileInfo {
void initXCOFFMCObjectFileInfo(const Triple &T);
void initDXContainerObjectFileInfo(const Triple &T);
MCSection *getDwarfComdatSection(const char *Name, uint64_t Hash) const;
-
-public:
- void setSDKVersion(const VersionTuple &TheSDKVersion) {
- SDKVersion = TheSDKVersion;
- }
-
- const VersionTuple &getSDKVersion() const { return SDKVersion; }
-
- void setDarwinTargetVariantTriple(const Triple &T) {
- DarwinTargetVariantTriple = T;
- }
-
- const Triple *getDarwinTargetVariantTriple() const {
- return DarwinTargetVariantTriple ? &*DarwinTargetVariantTriple : nullptr;
- }
-
- void setDarwinTargetVariantSDKVersion(const VersionTuple &TheSDKVersion) {
- DarwinTargetVariantSDKVersion = TheSDKVersion;
- }
-
- const VersionTuple &getDarwinTargetVariantSDKVersion() const {
- return DarwinTargetVariantSDKVersion;
- }
};
} // end namespace llvm
diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp
index 528caa12ec2126..2e3b67eca08c1d 100644
--- a/llvm/lib/MC/MCMachOStreamer.cpp
+++ b/llvm/lib/MC/MCMachOStreamer.cpp
@@ -533,14 +533,8 @@ MCStreamer *llvm::createMachOStreamer(MCContext &Context,
std::unique_ptr<MCCodeEmitter> &&CE,
bool DWARFMustBeAtTheEnd,
bool LabelSections) {
- MCMachOStreamer *S = new MCMachOStreamer(
- Context, std::move(MAB), std::move(OW), std::move(CE), LabelSections);
- const Triple &Target = Context.getTargetTriple();
- S->emitVersionForTarget(
- Target, Context.getObjectFileInfo()->getSDKVersion(),
- Context.getObjectFileInfo()->getDarwinTargetVariantTriple(),
- Context.getObjectFileInfo()->getDarwinTargetVariantSDKVersion());
- return S;
+ return new MCMachOStreamer(Context, std::move(MAB), std::move(OW),
+ std::move(CE), LabelSections);
}
// The AddrSig section uses a series of relocations to refer to the symbols that
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index d8f520ad02c2f2..fb36a88b9c7149 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -109,7 +109,6 @@ initializeRecordStreamer(const Module &M,
MCContext MCCtx(TT, MAI.get(), MRI.get(), STI.get(), &SrcMgr);
std::unique_ptr<MCObjectFileInfo> MOFI(
T->createMCObjectFileInfo(MCCtx, /*PIC=*/false));
- MOFI->setSDKVersion(M.getSDKVersion());
MCCtx.setObjectFileInfo(MOFI.get());
RecordStreamer Streamer(MCCtx, M);
T->createNullTargetStreamer(Streamer);
diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp
index 3a44ddf17974a4..e65b8f62369520 100644
--- a/llvm/tools/llvm-mc/llvm-mc.cpp
+++ b/llvm/tools/llvm-mc/llvm-mc.cpp
@@ -559,6 +559,9 @@ int main(int argc, char **argv) {
std::unique_ptr<MCCodeEmitter>(CE), *STI));
if (NoExecStack)
Str->initSections(true, *STI);
+ if (TheTriple.isOSBinFormatMachO() && TheTriple.isOSDarwin())
+ Str->emitVersionForTarget(TheTriple, VersionTuple(), nullptr,
+ VersionTuple());
}
int Res = 1;
|
|
@llvm/pr-subscribers-mc Author: Alexis Engelke (aengelke) ChangesThere's only a single user (MCMachOStreamer), so it makes more sense to move the version emission to the source of the data. (Not sure whether we want this, but I don't see benefits in storing this information in the MCObjectFileInfo.) Full diff: https://github.com/llvm/llvm-project/pull/103025.diff 5 Files Affected:
diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index b661a43c88b08d..78019d39742e64 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -489,10 +489,6 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr<MCObjectFileInfo> MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
- if (Opts.DarwinTargetVariantTriple)
- MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
- if (!Opts.DarwinTargetVariantSDKVersion.empty())
- MOFI->setDarwinTargetVariantSDKVersion(Opts.DarwinTargetVariantSDKVersion);
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.GenDwarfForAssembly)
@@ -574,6 +570,13 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
Str.reset(TheTarget->createMCObjectStreamer(
T, Ctx, std::move(MAB), std::move(OW), std::move(CE), *STI));
Str.get()->initSections(Opts.NoExecStack, *STI);
+ if (T.isOSBinFormatMachO() && T.isOSDarwin()) {
+ Triple *TVT = Opts.DarwinTargetVariantTriple
+ ? &*Opts.DarwinTargetVariantTriple
+ : nullptr;
+ Str->emitVersionForTarget(T, VersionTuple(), TVT,
+ Opts.DarwinTargetVariantSDKVersion);
+ }
}
// When -fembed-bitcode is passed to clang_as, a 1-byte marker
diff --git a/llvm/include/llvm/MC/MCObjectFileInfo.h b/llvm/include/llvm/MC/MCObjectFileInfo.h
index dda3e8a020f3ae..e2a2c84e47910b 100644
--- a/llvm/include/llvm/MC/MCObjectFileInfo.h
+++ b/llvm/include/llvm/MC/MCObjectFileInfo.h
@@ -458,9 +458,6 @@ class MCObjectFileInfo {
private:
bool PositionIndependent = false;
MCContext *Ctx = nullptr;
- VersionTuple SDKVersion;
- std::optional<Triple> DarwinTargetVariantTriple;
- VersionTuple DarwinTargetVariantSDKVersion;
void initMachOMCObjectFileInfo(const Triple &T);
void initELFMCObjectFileInfo(const Triple &T, bool Large);
@@ -471,29 +468,6 @@ class MCObjectFileInfo {
void initXCOFFMCObjectFileInfo(const Triple &T);
void initDXContainerObjectFileInfo(const Triple &T);
MCSection *getDwarfComdatSection(const char *Name, uint64_t Hash) const;
-
-public:
- void setSDKVersion(const VersionTuple &TheSDKVersion) {
- SDKVersion = TheSDKVersion;
- }
-
- const VersionTuple &getSDKVersion() const { return SDKVersion; }
-
- void setDarwinTargetVariantTriple(const Triple &T) {
- DarwinTargetVariantTriple = T;
- }
-
- const Triple *getDarwinTargetVariantTriple() const {
- return DarwinTargetVariantTriple ? &*DarwinTargetVariantTriple : nullptr;
- }
-
- void setDarwinTargetVariantSDKVersion(const VersionTuple &TheSDKVersion) {
- DarwinTargetVariantSDKVersion = TheSDKVersion;
- }
-
- const VersionTuple &getDarwinTargetVariantSDKVersion() const {
- return DarwinTargetVariantSDKVersion;
- }
};
} // end namespace llvm
diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp
index 528caa12ec2126..2e3b67eca08c1d 100644
--- a/llvm/lib/MC/MCMachOStreamer.cpp
+++ b/llvm/lib/MC/MCMachOStreamer.cpp
@@ -533,14 +533,8 @@ MCStreamer *llvm::createMachOStreamer(MCContext &Context,
std::unique_ptr<MCCodeEmitter> &&CE,
bool DWARFMustBeAtTheEnd,
bool LabelSections) {
- MCMachOStreamer *S = new MCMachOStreamer(
- Context, std::move(MAB), std::move(OW), std::move(CE), LabelSections);
- const Triple &Target = Context.getTargetTriple();
- S->emitVersionForTarget(
- Target, Context.getObjectFileInfo()->getSDKVersion(),
- Context.getObjectFileInfo()->getDarwinTargetVariantTriple(),
- Context.getObjectFileInfo()->getDarwinTargetVariantSDKVersion());
- return S;
+ return new MCMachOStreamer(Context, std::move(MAB), std::move(OW),
+ std::move(CE), LabelSections);
}
// The AddrSig section uses a series of relocations to refer to the symbols that
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index d8f520ad02c2f2..fb36a88b9c7149 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -109,7 +109,6 @@ initializeRecordStreamer(const Module &M,
MCContext MCCtx(TT, MAI.get(), MRI.get(), STI.get(), &SrcMgr);
std::unique_ptr<MCObjectFileInfo> MOFI(
T->createMCObjectFileInfo(MCCtx, /*PIC=*/false));
- MOFI->setSDKVersion(M.getSDKVersion());
MCCtx.setObjectFileInfo(MOFI.get());
RecordStreamer Streamer(MCCtx, M);
T->createNullTargetStreamer(Streamer);
diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp
index 3a44ddf17974a4..e65b8f62369520 100644
--- a/llvm/tools/llvm-mc/llvm-mc.cpp
+++ b/llvm/tools/llvm-mc/llvm-mc.cpp
@@ -559,6 +559,9 @@ int main(int argc, char **argv) {
std::unique_ptr<MCCodeEmitter>(CE), *STI));
if (NoExecStack)
Str->initSections(true, *STI);
+ if (TheTriple.isOSBinFormatMachO() && TheTriple.isOSDarwin())
+ Str->emitVersionForTarget(TheTriple, VersionTuple(), nullptr,
+ VersionTuple());
}
int Res = 1;
|
MaskRay
reviewed
Aug 13, 2024
llvm/tools/llvm-mc/llvm-mc.cpp
Outdated
| @@ -559,6 +559,9 @@ int main(int argc, char **argv) { | |||
| std::unique_ptr<MCCodeEmitter>(CE), *STI)); | |||
| if (NoExecStack) | |||
| Str->initSections(true, *STI); | |||
| if (TheTriple.isOSBinFormatMachO() && TheTriple.isOSDarwin()) | |||
There was a problem hiding this comment.
Is this needed? It seems that emitVersionForTarget will ignore a major version of 0.
There was a problem hiding this comment.
No. I just put it in front of all emitVersionForTarget for clarity.
MaskRay
reviewed
Aug 13, 2024
| @@ -109,7 +109,6 @@ initializeRecordStreamer(const Module &M, | |||
| MCContext MCCtx(TT, MAI.get(), MRI.get(), STI.get(), &SrcMgr); | |||
| std::unique_ptr<MCObjectFileInfo> MOFI( | |||
| T->createMCObjectFileInfo(MCCtx, /*PIC=*/false)); | |||
| MOFI->setSDKVersion(M.getSDKVersion()); | |||
There was a problem hiding this comment.
Regular and Thin LTO use ModuleSymbolTable to record symbols in the IR file (llvm-nm a.bc). Will this cause a behavior difference?
There was a problem hiding this comment.
I don't think so? I would expect that the information is always stored in the module -- and all call sites that create a MCObjectStreamer take the information from the module (the only reader of the MOFI fields was createMachOStreamer, but emitVersionForTarget is always (also) called from the AsmPrinter). I'm just 95% confident: tests pass + local code changes seem reasonable to me, but I certainly miss the "big picture" regarding LTO.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's only a single user (MCMachOStreamer), so it makes more sense to move the version emission to the source of the data.
(Not sure whether we want this, but I don't see benefits in storing this information in the MCObjectFileInfo.)