Skip to content

IR: Store the default subtarget info in named metadata instead of the context. #98673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/lib/CodeGen/BackendUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,8 @@ void clang::EmitBackendOutput(
}
}

M->setDefaultSubtargetInfo(TOpts.CPU, llvm::join(TOpts.Features, ","));

EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M, VFS);
AsmHelper.EmitAssembly(Action, std::move(OS), BC);

Expand Down
6 changes: 0 additions & 6 deletions clang/lib/CodeGen/CodeGenAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
Ctx.setDiagnosticHandler(std::make_unique<ClangDiagnosticHandler>(
CodeGenOpts, this));

Ctx.setDefaultTargetCPU(TargetOpts.CPU);
Ctx.setDefaultTargetFeatures(llvm::join(TargetOpts.Features, ","));

Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr =
setupLLVMOptimizationRemarks(
Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses,
Expand Down Expand Up @@ -1203,9 +1200,6 @@ void CodeGenAction::ExecuteAction() {
Ctx.setDiagnosticHandler(
std::make_unique<ClangDiagnosticHandler>(CodeGenOpts, &Result));

Ctx.setDefaultTargetCPU(TargetOpts.CPU);
Ctx.setDefaultTargetFeatures(llvm::join(TargetOpts.Features, ","));

Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr =
setupLLVMOptimizationRemarks(
Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses,
Expand Down
16 changes: 0 additions & 16 deletions llvm/include/llvm/IR/LLVMContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,22 +327,6 @@ class LLVMContext {
[[deprecated("Always returns false")]]
bool supportsTypedPointers() const;

/// Get or set the current "default" target CPU (target-cpu function
/// attribute). The intent is that compiler frontends will set this to a value
/// that reflects the attribute that a function would get "by default" without
/// any specific function attributes, and compiler passes will attach the
/// attribute to newly created functions that are not associated with a
/// particular function, such as global initializers.
/// Function::createWithDefaultAttr() will create functions with this
/// attribute. This function should only be called by passes that run at
/// compile time and not by the backend or LTO passes.
StringRef getDefaultTargetCPU();
void setDefaultTargetCPU(StringRef CPU);

/// Similar to {get,set}DefaultTargetCPU() but for default target-features.
StringRef getDefaultTargetFeatures();
void setDefaultTargetFeatures(StringRef Features);

private:
// Module needs access to the add/removeModule methods.
friend class Module;
Expand Down
18 changes: 18 additions & 0 deletions llvm/include/llvm/IR/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,24 @@ class LLVM_EXTERNAL_VISIBILITY Module {

/// Set the target variant version build SDK version metadata.
void setDarwinTargetVariantSDKVersion(VersionTuple Version);

/// Get the current "default" target CPU (target-cpu function
/// attribute). The intent is that compiler frontends will set this to a value
/// that reflects the attribute that a function would get "by default" without
/// any specific function attributes, and compiler passes will attach the
/// attribute to newly created functions that are not associated with a
/// particular function, such as global initializers.
/// Function::createWithDefaultAttr() will create functions with this
/// attribute. This function should only be called by passes that run at
/// compile time and not by the backend or LTO passes.
StringRef getDefaultTargetCPU();

/// Similar to getDefaultTargetCPU() but for default target-features.
StringRef getDefaultTargetFeatures();

/// Set the data returned by getDefaultTargetCPU() and
/// getDefaultTargetFeatures().
void setDefaultSubtargetInfo(StringRef CPU, StringRef Features);
};

/// Given "llvm.used" or "llvm.compiler.used" as a global name, collect the
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/IR/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,10 @@ Function *Function::createWithDefaultAttr(FunctionType *Ty,
}
if (M->getModuleFlag("function_return_thunk_extern"))
B.addAttribute(Attribute::FnRetThunkExtern);
StringRef DefaultCPU = F->getContext().getDefaultTargetCPU();
StringRef DefaultCPU = M->getDefaultTargetCPU();
if (!DefaultCPU.empty())
B.addAttribute("target-cpu", DefaultCPU);
StringRef DefaultFeatures = F->getContext().getDefaultTargetFeatures();
StringRef DefaultFeatures = M->getDefaultTargetFeatures();
if (!DefaultFeatures.empty())
B.addAttribute("target-features", DefaultFeatures);
F->addFnAttrs(B);
Expand Down
16 changes: 0 additions & 16 deletions llvm/lib/IR/LLVMContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,19 +390,3 @@ void LLVMContext::setOpaquePointers(bool Enable) const {
bool LLVMContext::supportsTypedPointers() const {
return false;
}

StringRef LLVMContext::getDefaultTargetCPU() {
return pImpl->DefaultTargetCPU;
}

void LLVMContext::setDefaultTargetCPU(StringRef CPU) {
pImpl->DefaultTargetCPU = CPU;
}

StringRef LLVMContext::getDefaultTargetFeatures() {
return pImpl->DefaultTargetFeatures;
}

void LLVMContext::setDefaultTargetFeatures(StringRef Features) {
pImpl->DefaultTargetFeatures = Features;
}
35 changes: 35 additions & 0 deletions llvm/lib/IR/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/RandomNumberGenerator.h"
#include "llvm/Support/VersionTuple.h"
#include "llvm/TargetParser/Triple.h"
#include <algorithm>
#include <cassert>
#include <cstdint>
Expand Down Expand Up @@ -903,3 +904,37 @@ VersionTuple Module::getDarwinTargetVariantSDKVersion() const {
void Module::setDarwinTargetVariantSDKVersion(VersionTuple Version) {
addSDKVersionMD(Version, *this, "darwin.target_variant.SDK Version");
}

void Module::setDefaultSubtargetInfo(StringRef CPU, StringRef Features) {
auto *MD = getOrInsertNamedMetadata("llvm.subtarget.info");
MD->clearOperands();
MD->addOperand(
MDNode::get(getContext(), {MDString::get(getContext(), CPU),
MDString::get(getContext(), Features)}));
}

StringRef Module::getDefaultTargetCPU() {
auto *MD = getNamedMetadata("llvm.subtarget.info");
// FIXME: This function is only meant to be called pre-LTO, but the AMDGPU and
// NVPTX backends end up calling this function in their CtorDtorLowering
// passes. For now we just return an empty string to avoid breaking them.
if (!MD) {
Triple T(getTargetTriple());
if (T.isAMDGPU() || T.isNVPTX())
return "";
report_fatal_error("Invalid call to getDefaultTargetCPU()");
}
return cast<MDString>(MD->getOperand(0)->getOperand(0))->getString();
}

StringRef Module::getDefaultTargetFeatures() {
auto *MD = getNamedMetadata("llvm.subtarget.info");
// FIXME: See getDefaultTargetCPU().
if (!MD) {
Triple T(getTargetTriple());
if (T.isAMDGPU() || T.isNVPTX())
return "";
report_fatal_error("Invalid call to getDefaultTargetFeatures()");
}
return cast<MDString>(MD->getOperand(0)->getOperand(1))->getString();
}
11 changes: 11 additions & 0 deletions llvm/lib/Linker/IRMover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1240,11 +1240,22 @@ void IRLinker::linkNamedMDNodes() {
if (IsPerformingImport && NMD.getName() == "llvm.stats")
continue;

// Default subtarget info is only intended to be used before LTO and
// shouldn't be present after merging because the default subtargets may be
// different. Even if they were the same we wouldn't want to keep them to
// avoid introducing bugs that would only occur when merging modules with
// different subtarget info.
if (NMD.getName() == "llvm.subtarget.info")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an overly simplistic take. Downstream we need a subset of the target features during LTO, and it wouldn't surprise me if there are cases upstream that could benefit from it too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such cases should use a separate field, such as the e_flags field that I suggested in the Discourse thread. That would more cleanly implement the semantics that the target actually wants. The metadata that I'm adding here is intended as a generic description of the singular default target CPU/features, and since that's not a concept that exists generically after LTO it gets removed from the IR at that point.

continue;

NamedMDNode *DestNMD = DstM.getOrInsertNamedMetadata(NMD.getName());
// Add Src elements into Dest node.
for (const MDNode *Op : NMD.operands())
DestNMD->addOperand(Mapper.mapMDNode(*Op));
}

if (auto *NMD = DstM.getNamedMetadata("llvm.subtarget.info"))
DstM.eraseNamedMetadata(NMD);
}

/// Merge the linker flags in Src into the Dest module.
Expand Down
Loading