Skip to content
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.

Sign up for GitHub

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

[NFC][CodeGen] Change CodeGenOpt::Level/CodeGenFileType into enum classes #66295

Merged
merged 4 commits into from
Sep 14, 2023
Merged

[NFC][CodeGen] Change CodeGenOpt::Level/CodeGenFileType into enum classes #66295

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
855e4ce
[NFC][CodeGen] Change CodeGenOpt::{Level,FileType} into enum classes
aeubanks Sep 13, 2023
5d1657b
Use CodeGenOptLevel instead of CodeGenOpt::Level
aeubanks Sep 13, 2023
a131eb9
flang/mlir fixes
aeubanks Sep 14, 2023
9cd29db
Revert libcxxabi change
aeubanks Sep 14, 2023
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
12 changes: 6 additions & 6 deletions clang/lib/CodeGen/BackendUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,12 @@ getCodeModel(const CodeGenOptions &CodeGenOpts) {

static CodeGenFileType getCodeGenFileType(BackendAction Action) {
if (Action == Backend_EmitObj)
return CGFT_ObjectFile;
return CodeGenFileType::ObjectFile;
else if (Action == Backend_EmitMCNull)
return CGFT_Null;
return CodeGenFileType::Null;
else {
assert(Action == Backend_EmitAssembly && "Invalid action!");
return CGFT_AssemblyFile;
return CodeGenFileType::AssemblyFile;
}
}

Expand Down Expand Up @@ -561,10 +561,10 @@ void EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
std::string FeaturesStr =
llvm::join(TargetOpts.Features.begin(), TargetOpts.Features.end(), ",");
llvm::Reloc::Model RM = CodeGenOpts.RelocationModel;
std::optional<CodeGenOpt::Level> OptLevelOrNone =
std::optional<CodeGenOptLevel> OptLevelOrNone =
CodeGenOpt::getLevel(CodeGenOpts.OptimizationLevel);
assert(OptLevelOrNone && "Invalid optimization level!");
CodeGenOpt::Level OptLevel = *OptLevelOrNone;
CodeGenOptLevel OptLevel = *OptLevelOrNone;

llvm::TargetOptions Options;
if (!initTargetOptions(Diags, Options, CodeGenOpts, TargetOpts, LangOpts,
Expand Down Expand Up @@ -1216,7 +1216,7 @@ static void runThinLTOBackend(
Conf.CodeModel = getCodeModel(CGOpts);
Conf.MAttrs = TOpts.Features;
Conf.RelocModel = CGOpts.RelocationModel;
std::optional<CodeGenOpt::Level> OptLevelOrNone =
std::optional<CodeGenOptLevel> OptLevelOrNone =
CodeGenOpt::getLevel(CGOpts.OptimizationLevel);
assert(OptLevelOrNone && "Invalid optimization level!");
Conf.CGOptLevel = *OptLevelOrNone;
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,27 +661,27 @@ static bool FixupInvocation(CompilerInvocation &Invocation,

static unsigned getOptimizationLevel(ArgList &Args, InputKind IK,
DiagnosticsEngine &Diags) {
unsigned DefaultOpt = llvm::CodeGenOpt::None;
unsigned DefaultOpt = 0;
if ((IK.getLanguage() == Language::OpenCL ||
IK.getLanguage() == Language::OpenCLCXX) &&
!Args.hasArg(OPT_cl_opt_disable))
DefaultOpt = llvm::CodeGenOpt::Default;
DefaultOpt = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not keen on having these as magic numbers. Surely you can
static_cast<unsigned>(CodeGenOptLevel::Default) or whatever.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Otherwise the the default is effectively defined twice in two different places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

llvm::CodeGenOpt is for LLVM's codegen pipeline, but here it's getting overloaded to be used in clang's IR codegen. These are two different concepts where the defaults happen to have the same value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that different; CodeGenOpt::getLevel() just uses a static_cast to implement the inverse mapping. This direction of the conversion should be consistent IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do eventually map the opt level here to a CodeGenOpt via CodeGenOpt::getLevel(), but we also use the opt level here for a bunch of other things in the clang codegen process. So whatever the "default" llvm codegen pipeline opt level is shouldn't be tied to a clang frontend opt level default.

Maybe I'm not understanding your comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually it looks to me like clang only cares about 0/not-0, and the specific non-zero number is irrelevant. So, internally clang codegen would be better off with a simple Optimize bool. (It looks like LangOpts does it this way already.) CodeGenOpts probably does need to track the actual user-specified level to pass down to LLVM, but it doesn't need it for its own purposes.


if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
if (A->getOption().matches(options::OPT_O0))
return llvm::CodeGenOpt::None;
return 0;

if (A->getOption().matches(options::OPT_Ofast))
return llvm::CodeGenOpt::Aggressive;
return 3;

assert(A->getOption().matches(options::OPT_O));

StringRef S(A->getValue());
if (S == "s" || S == "z")
return llvm::CodeGenOpt::Default;
return 2;

if (S == "g")
return llvm::CodeGenOpt::Less;
return 1;

return getLastArgIntValue(Args, OPT_O, DefaultOpt, Diags);
}
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Interpreter/DeviceOffload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ llvm::Expected<llvm::StringRef> IncrementalCUDADeviceParser::GeneratePTX() {

llvm::legacy::PassManager PM;
if (TargetMachine->addPassesToEmitFile(PM, dest, nullptr,
llvm::CGFT_AssemblyFile)) {
llvm::CodeGenFileType::AssemblyFile)) {
return llvm::make_error<llvm::StringError>(
"NVPTX backend cannot produce PTX code.",
llvm::inconvertibleErrorCode());
Expand Down
25 changes: 12 additions & 13 deletions clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ using namespace llvm;
typedef void (*LLVMFunc)(int*, int*, int*, int);

// Helper function to parse command line args and find the optimization level
static CodeGenOpt::Level
getOptLevel(const std::vector<const char *> &ExtraArgs) {
static CodeGenOptLevel getOptLevel(const std::vector<const char *> &ExtraArgs) {
// Find the optimization level from the command line args
CodeGenOpt::Level OLvl = CodeGenOpt::Default;
CodeGenOptLevel OLvl = CodeGenOptLevel::Default;
for (auto &A : ExtraArgs) {
if (A[0] == '-' && A[1] == 'O') {
if (auto Level = CodeGenOpt::parseLevel(A[2])) {
Expand All @@ -73,19 +72,19 @@ static void ErrorAndExit(std::string message) {
// Helper function to add optimization passes to the TargetMachine at the
// specified optimization level, OptLevel
static void RunOptimizationPasses(raw_ostream &OS, Module &M,
CodeGenOpt::Level OptLevel) {
CodeGenOptLevel OptLevel) {
llvm::OptimizationLevel OL;
switch (OptLevel) {
case CodeGenOpt::None:
case CodeGenOptLevel::None:
OL = OptimizationLevel::O0;
break;
case CodeGenOpt::Less:
case CodeGenOptLevel::Less:
OL = OptimizationLevel::O1;
break;
case CodeGenOpt::Default:
case CodeGenOptLevel::Default:
OL = OptimizationLevel::O2;
break;
case CodeGenOpt::Aggressive:
case CodeGenOptLevel::Aggressive:
OL = OptimizationLevel::O3;
break;
}
Expand All @@ -110,7 +109,7 @@ static void RunOptimizationPasses(raw_ostream &OS, Module &M,
}

// Mimics the opt tool to run an optimization pass over the provided IR
static std::string OptLLVM(const std::string &IR, CodeGenOpt::Level OLvl) {
static std::string OptLLVM(const std::string &IR, CodeGenOptLevel OLvl) {
// Create a module that will run the optimization passes
SMDiagnostic Err;
LLVMContext Context;
Expand Down Expand Up @@ -154,7 +153,7 @@ static void RunFuncOnInputs(LLVMFunc f, int Arr[kNumArrays][kArraySize]) {
}

// Takes a string of IR and compiles it using LLVM's JIT Engine
static void CreateAndRunJITFunc(const std::string &IR, CodeGenOpt::Level OLvl) {
static void CreateAndRunJITFunc(const std::string &IR, CodeGenOptLevel OLvl) {
SMDiagnostic Err;
LLVMContext Context;
std::unique_ptr<Module> M = parseIR(MemoryBufferRef(IR, "IR"), Err, Context);
Expand Down Expand Up @@ -205,7 +204,7 @@ static void CreateAndRunJITFunc(const std::string &IR, CodeGenOpt::Level OLvl) {
#endif

// Figure out if we are running the optimized func or the unoptimized func
RunFuncOnInputs(f, (OLvl == CodeGenOpt::None) ? UnoptArrays : OptArrays);
RunFuncOnInputs(f, (OLvl == CodeGenOptLevel::None) ? UnoptArrays : OptArrays);

EE->runStaticConstructorsDestructors(true);
}
Expand All @@ -219,13 +218,13 @@ void clang_fuzzer::HandleLLVM(const std::string &IR,
memcpy(UnoptArrays, InputArrays, kTotalSize);

// Parse ExtraArgs to set the optimization level
CodeGenOpt::Level OLvl = getOptLevel(ExtraArgs);
CodeGenOptLevel OLvl = getOptLevel(ExtraArgs);

// First we optimize the IR by running a loop vectorizer pass
std::string OptIR = OptLLVM(IR, OLvl);

CreateAndRunJITFunc(OptIR, OLvl);
CreateAndRunJITFunc(IR, CodeGenOpt::None);
CreateAndRunJITFunc(IR, CodeGenOptLevel::None);

if (memcmp(OptArrays, UnoptArrays, kTotalSize))
ErrorAndExit("!!!BUG!!!");
Expand Down
10 changes: 6 additions & 4 deletions clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ std::unique_ptr<lto::LTO> createLTO(

StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "O2");
Conf.MAttrs = Features;
std::optional<CodeGenOpt::Level> CGOptLevelOrNone =
std::optional<CodeGenOptLevel> CGOptLevelOrNone =
CodeGenOpt::parseLevel(OptLevel[1]);
assert(CGOptLevelOrNone && "Invalid optimization level");
Conf.CGOptLevel = *CGOptLevelOrNone;
Expand Down Expand Up @@ -569,8 +569,9 @@ std::unique_ptr<lto::LTO> createLTO(
};
}
Conf.PostOptModuleHook = Hook;
Conf.CGFileType =
(Triple.isNVPTX() || SaveTemps) ? CGFT_AssemblyFile : CGFT_ObjectFile;
Conf.CGFileType = (Triple.isNVPTX() || SaveTemps)
? CodeGenFileType::AssemblyFile
: CodeGenFileType::ObjectFile;

// TODO: Handle remark files
Conf.HasWholeProgramVisibility = Args.hasArg(OPT_whole_program);
Expand Down Expand Up @@ -840,7 +841,8 @@ Expected<StringRef> compileModule(Module &M) {
legacy::PassManager CodeGenPasses;
TargetLibraryInfoImpl TLII(Triple(M.getTargetTriple()));
CodeGenPasses.add(new TargetLibraryInfoWrapperPass(TLII));
if (TM->addPassesToEmitFile(CodeGenPasses, *OS, nullptr, CGFT_ObjectFile))
if (TM->addPassesToEmitFile(CodeGenPasses, *OS, nullptr,
CodeGenFileType::ObjectFile))
return createStringError(inconvertibleErrorCode(),
"Failed to execute host backend");
CodeGenPasses.run(M);
Expand Down
4 changes: 2 additions & 2 deletions flang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ static bool parseShowColorsArgs(const llvm::opt::ArgList &args,
/// Extracts the optimisation level from \a args.
static unsigned getOptimizationLevel(llvm::opt::ArgList &args,
clang::DiagnosticsEngine &diags) {
unsigned defaultOpt = llvm::CodeGenOpt::None;
unsigned defaultOpt = llvm::CodeGenOptLevel::None;

if (llvm::opt::Arg *a =
args.getLastArg(clang::driver::options::OPT_O_Group)) {
if (a->getOption().matches(clang::driver::options::OPT_O0))
return llvm::CodeGenOpt::None;
return llvm::CodeGenOptLevel::None;

assert(a->getOption().matches(clang::driver::options::OPT_O));

Expand Down
11 changes: 6 additions & 5 deletions flang/lib/Frontend/FrontendActions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,10 +777,10 @@ bool CodeGenAction::setUpTargetMachine() {

// Create `TargetMachine`
const auto &CGOpts = ci.getInvocation().getCodeGenOpts();
std::optional<llvm::CodeGenOpt::Level> OptLevelOrNone =
std::optional<llvm::CodeGenOptLevel> OptLevelOrNone =
llvm::CodeGenOpt::getLevel(CGOpts.OptimizationLevel);
assert(OptLevelOrNone && "Invalid optimization level!");
llvm::CodeGenOpt::Level OptLevel = *OptLevelOrNone;
llvm::CodeGenOptLevel OptLevel = *OptLevelOrNone;
std::string featuresStr = getTargetFeatures(ci);
tm.reset(theTarget->createTargetMachine(
theTriple, /*CPU=*/targetOpts.cpu,
Expand Down Expand Up @@ -847,9 +847,10 @@ static void generateMachineCodeOrAssemblyImpl(clang::DiagnosticsEngine &diags,
assert(tlii && "Failed to create TargetLibraryInfo");
codeGenPasses.add(new llvm::TargetLibraryInfoWrapperPass(*tlii));

llvm::CodeGenFileType cgft = (act == BackendActionTy::Backend_EmitAssembly)
? llvm::CodeGenFileType::CGFT_AssemblyFile
: llvm::CodeGenFileType::CGFT_ObjectFile;
llvm::CodeGenFileType cgft =
(act == BackendActionTy::Backend_EmitAssembly)
? llvm::CodeGenFileType::CodeGenFileType::AssemblyFile
: llvm::CodeGenFileType::CodeGenFileType::ObjectFile;
if (tm.addPassesToEmitFile(codeGenPasses, os, nullptr, cgft)) {
unsigned diagID =
diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
Expand Down
Loading