-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Overhaul the TargetMachine and LLVMTargetMachine Classes #111234
base: main
Are you sure you want to change the base?
Changes from 2 commits
8b28dd9
aa053ec
38aad30
6db024e
b52f940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
//===-- CodeGenCommonTMImpl.h -----------------------------------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
/// | ||
/// \file This file describes the CodeGenCommonTMImpl class, which | ||
/// implements a set of functionality used by \c TargetMachine classes in | ||
/// LLVM that make use of the target-independent code generator. | ||
//===----------------------------------------------------------------------===// | ||
#ifndef LLVM_CODEGEN_CODEGENCOMMONTMIMPL_H | ||
#define LLVM_CODEGEN_CODEGENCOMMONTMIMPL_H | ||
#include "llvm/Target/TargetMachine.h" | ||
|
||
namespace llvm { | ||
|
||
/// \brief implements a set of functionality in the \c TargetMachine class | ||
/// for targets that make use of the independent code generator (CodeGen) | ||
/// library. Must not be used directly in code unless to inherit its | ||
/// implementation. | ||
class CodeGenCommonTMImpl : public TargetMachine { | ||
protected: // Can only create subclasses. | ||
CodeGenCommonTMImpl(const Target &T, StringRef DataLayoutString, | ||
const Triple &TT, StringRef CPU, StringRef FS, | ||
const TargetOptions &Options, Reloc::Model RM, | ||
CodeModel::Model CM, CodeGenOptLevel OL); | ||
|
||
void initAsmInfo(); | ||
|
||
public: | ||
/// Get a TargetTransformInfo implementation for the target. | ||
/// | ||
/// The TTI returned uses the common code generator to answer queries about | ||
/// the IR. | ||
TargetTransformInfo getTargetTransformInfo(const Function &F) const override; | ||
Comment on lines
+33
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TargetTransformInfo isn't really part of the CodeGen interface. This should probably move up to TargetMachine. I see that TTI does make a little use of TM. Seems to be for managing subtarget instances, plus a few of the odd fields that really should come from the module There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's because it's already in the There's no function in |
||
|
||
/// Create a pass configuration object to be used by addPassToEmitX methods | ||
/// for generating a pipeline of CodeGen passes. | ||
virtual TargetPassConfig *createPassConfig(PassManagerBase &PM) override; | ||
|
||
/// Add passes to the specified pass manager to get the specified file | ||
/// emitted. Typically this will involve several steps of code generation. | ||
/// \p MMIWP is an optional parameter that, if set to non-nullptr, | ||
/// will be used to set the MachineModuloInfo for this PM. | ||
bool | ||
addPassesToEmitFile(PassManagerBase &PM, raw_pwrite_stream &Out, | ||
raw_pwrite_stream *DwoOut, CodeGenFileType FileType, | ||
bool DisableVerify = true, | ||
MachineModuleInfoWrapperPass *MMIWP = nullptr) override; | ||
|
||
/// Add passes to the specified pass manager to get machine code emitted with | ||
/// the MCJIT. This method returns true if machine code is not supported. It | ||
/// fills the MCContext Ctx pointer which can be used to build custom | ||
/// MCStreamer. | ||
bool addPassesToEmitMC(PassManagerBase &PM, MCContext *&Ctx, | ||
raw_pwrite_stream &Out, | ||
bool DisableVerify = true) override; | ||
|
||
/// Adds an AsmPrinter pass to the pipeline that prints assembly or | ||
/// machine code from the MI representation. | ||
bool addAsmPrinter(PassManagerBase &PM, raw_pwrite_stream &Out, | ||
raw_pwrite_stream *DwoOut, CodeGenFileType FileType, | ||
MCContext &Context) override; | ||
|
||
Expected<std::unique_ptr<MCStreamer>> | ||
createMCStreamer(raw_pwrite_stream &Out, raw_pwrite_stream *DwoOut, | ||
CodeGenFileType FileType, MCContext &Ctx) override; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In #108690 I'm also trying to use callbacks to avoid these functions in new pass manager. |
||
}; | ||
|
||
/// Helper method for getting the code model, returning Default if | ||
/// CM does not have a value. The tiny and kernel models will produce | ||
/// an error, so targets that support them or require more complex codemodel | ||
/// selection logic should implement and call their own getEffectiveCodeModel. | ||
inline CodeModel::Model | ||
getEffectiveCodeModel(std::optional<CodeModel::Model> CM, | ||
CodeModel::Model Default) { | ||
if (CM) { | ||
// By default, targets do not support the tiny and kernel models. | ||
if (*CM == CodeModel::Tiny) | ||
report_fatal_error("Target does not support the tiny CodeModel", false); | ||
if (*CM == CodeModel::Kernel) | ||
report_fatal_error("Target does not support the kernel CodeModel", false); | ||
return *CM; | ||
} | ||
return Default; | ||
} | ||
|
||
} // namespace llvm | ||
|
||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing end of file line There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be fixed, but please check this again. My editor either doesn't show it to me or puts too many whitespaces at the end, causing clang format to complain. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,7 @@ class DILocation; | |
class Function; | ||
class GISelChangeObserver; | ||
class GlobalValue; | ||
class LLVMTargetMachine; | ||
class TargetMachine; | ||
class MachineConstantPool; | ||
class MachineFrameInfo; | ||
class MachineFunction; | ||
|
@@ -256,7 +256,7 @@ struct LandingPadInfo { | |
|
||
class LLVM_ABI MachineFunction { | ||
Function &F; | ||
const LLVMTargetMachine &Target; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd expect all of the references in the codegen infrastructure to refer to the codegen dependent subclass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's a need anymore. I'm hoping In other words, a Target can just ignore everything in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're in MachineFunction, you are definitely using CodeGen. So why wouldn't MachineFunction reference the lowest common subclass for codegen? |
||
const TargetMachine &Target; | ||
const TargetSubtargetInfo *STI; | ||
MCContext &Ctx; | ||
|
||
|
@@ -634,7 +634,7 @@ class LLVM_ABI MachineFunction { | |
/// for instructions that have a stack spill fused into them. | ||
const static unsigned int DebugOperandMemNumber; | ||
|
||
MachineFunction(Function &F, const LLVMTargetMachine &Target, | ||
MachineFunction(Function &F, const TargetMachine &Target, | ||
const TargetSubtargetInfo &STI, MCContext &Ctx, | ||
unsigned FunctionNum); | ||
MachineFunction(const MachineFunction &) = delete; | ||
|
@@ -707,7 +707,7 @@ class LLVM_ABI MachineFunction { | |
void assignBeginEndSections(); | ||
|
||
/// getTarget - Return the target machine this machine code is compiled with | ||
const LLVMTargetMachine &getTarget() const { return Target; } | ||
const TargetMachine &getTarget() const { return Target; } | ||
|
||
/// getSubtarget - Return the subtarget for which this machine code is being | ||
/// compiled. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this name needs work. CodeGenTargetMachine maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I named it
CodeGenTargetMachine
, but I picked an ugly name on purpose to remind people not to use this class directly in their code unless they are subclassing it for their targets.Again, my goal was to remove the need to static cast every time you need a CodeGen-specific feature of the TargetMachine, it's already in the interface. The target of choice has the freedom to not use it and either a) start from scratch in order to use the target-independent code generator, or b) use its own code generation scheme.
This is essentially the best middle ground I found between completely removing
LLVMTargetMachine
and not breaking the compilation process. With this change, it's as ifLLVMTargetMachine
was removed from the user's perspective. Meanwhile, this shouldn't cause any havoc on the toolchain compilation process, as all library requirements remain the same (i.e. there's no requirement to link to CodeGen unless you're using the CodeGen-related TM interfaces).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still should probably go for the less ugly name (at least the TM part)