Skip to content

Commit 39efaa0

Browse files
gbaraldigbaraldi
committed
Adress comments from code review
1 parent a1320e3 commit 39efaa0

File tree

2 files changed

+83
-60
lines changed

2 files changed

+83
-60
lines changed

‎llvm/lib/Passes/PassBuilder.cpp‎

Lines changed: 55 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,26 @@ class RequireAllMachineFunctionPropertiesPass
480480

481481
} // namespace
482482

483+
static std::optional<OptimizationLevel> parseOptLevel(StringRef S) {
484+
return StringSwitch<std::optional<OptimizationLevel>>(S)
485+
.Case("O0", OptimizationLevel::O0)
486+
.Case("O1", OptimizationLevel::O1)
487+
.Case("O2", OptimizationLevel::O2)
488+
.Case("O3", OptimizationLevel::O3)
489+
.Case("Os", OptimizationLevel::Os)
490+
.Case("Oz", OptimizationLevel::Oz)
491+
.Default(std::nullopt);
492+
}
493+
494+
static Expected<OptimizationLevel> parseOptLevelParam(StringRef S) {
495+
std::optional<OptimizationLevel> OptLevel = parseOptLevel(S);
496+
if (OptLevel)
497+
return *OptLevel;
498+
return make_error<StringError>(
499+
formatv("invalid optimization level '{}'", S).str(),
500+
inconvertibleErrorCode());
501+
}
502+
483503
PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
484504
std::optional<PGOOptions> PGOOpt,
485505
PassInstrumentationCallbacks *PIC)
@@ -529,31 +549,18 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
529549
#include "llvm/Passes/MachinePassRegistry.def"
530550
});
531551
}
532-
auto parseLevelParam = [](StringRef P) -> Expected<OptimizationLevel> {
533-
auto OptLevel = llvm::StringSwitch<std::optional<OptimizationLevel>>(P)
534-
.Case("O0", OptimizationLevel::O0)
535-
.Case("O1", OptimizationLevel::O1)
536-
.Case("O2", OptimizationLevel::O2)
537-
.Case("O3", OptimizationLevel::O3)
538-
.Case("Os", OptimizationLevel::Os)
539-
.Case("Oz", OptimizationLevel::Oz)
540-
.Default(std::nullopt);
541-
if (!OptLevel)
542-
return llvm::createStringError(llvm::inconvertibleErrorCode(),
543-
"invalid optimization level '%s'",
544-
P.str().c_str());
545-
return *OptLevel;
546-
};
547552

548553
// Module-level callbacks without LTO phase
549554
registerPipelineParsingCallback(
550-
[this, parseLevelParam](StringRef Name, ModulePassManager &PM,
551-
ArrayRef<PassBuilder::PipelineElement>) {
555+
[this](StringRef Name, ModulePassManager &PM,
556+
ArrayRef<PassBuilder::PipelineElement>) {
552557
#define MODULE_CALLBACK(NAME, INVOKE) \
553558
if (PassBuilder::checkParametrizedPassName(Name, NAME)) { \
554-
auto L = PassBuilder::parsePassParameters(parseLevelParam, Name, NAME); \
555-
if (!L) \
556-
return (errs() << NAME ": " << toString(L.takeError()) << '\n', false); \
559+
auto L = PassBuilder::parsePassParameters(parseOptLevelParam, Name, NAME); \
560+
if (!L) { \
561+
errs() << NAME ": " << toString(L.takeError()) << '\n'; \
562+
return false; \
563+
} \
557564
INVOKE(PM, L.get()); \
558565
return true; \
559566
}
@@ -564,13 +571,15 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
564571

565572
// Module-level callbacks with LTO phase (use Phase::None for string API)
566573
registerPipelineParsingCallback(
567-
[this, parseLevelParam](StringRef Name, ModulePassManager &PM,
568-
ArrayRef<PassBuilder::PipelineElement>) {
574+
[this](StringRef Name, ModulePassManager &PM,
575+
ArrayRef<PassBuilder::PipelineElement>) {
569576
#define MODULE_LTO_CALLBACK(NAME, INVOKE) \
570577
if (PassBuilder::checkParametrizedPassName(Name, NAME)) { \
571-
auto L = PassBuilder::parsePassParameters(parseLevelParam, Name, NAME); \
572-
if (!L) \
573-
return (errs() << NAME ": " << toString(L.takeError()) << '\n', false); \
578+
auto L = PassBuilder::parsePassParameters(parseOptLevelParam, Name, NAME); \
579+
if (!L) { \
580+
errs() << NAME ": " << toString(L.takeError()) << '\n'; \
581+
return false; \
582+
} \
574583
INVOKE(PM, L.get(), ThinOrFullLTOPhase::None); \
575584
return true; \
576585
}
@@ -581,13 +590,15 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
581590

582591
// Function-level callbacks
583592
registerPipelineParsingCallback(
584-
[this, parseLevelParam](StringRef Name, FunctionPassManager &PM,
585-
ArrayRef<PassBuilder::PipelineElement>) {
593+
[this](StringRef Name, FunctionPassManager &PM,
594+
ArrayRef<PassBuilder::PipelineElement>) {
586595
#define FUNCTION_CALLBACK(NAME, INVOKE) \
587596
if (PassBuilder::checkParametrizedPassName(Name, NAME)) { \
588-
auto L = PassBuilder::parsePassParameters(parseLevelParam, Name, NAME); \
589-
if (!L) \
590-
return (errs() << NAME ": " << toString(L.takeError()) << '\n', false); \
597+
auto L = PassBuilder::parsePassParameters(parseOptLevelParam, Name, NAME); \
598+
if (!L) { \
599+
errs() << NAME ": " << toString(L.takeError()) << '\n'; \
600+
return false; \
601+
} \
591602
INVOKE(PM, L.get()); \
592603
return true; \
593604
}
@@ -598,13 +609,15 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
598609

599610
// CGSCC-level callbacks
600611
registerPipelineParsingCallback(
601-
[this, parseLevelParam](StringRef Name, CGSCCPassManager &PM,
602-
ArrayRef<PassBuilder::PipelineElement>) {
612+
[this](StringRef Name, CGSCCPassManager &PM,
613+
ArrayRef<PassBuilder::PipelineElement>) {
603614
#define CGSCC_CALLBACK(NAME, INVOKE) \
604615
if (PassBuilder::checkParametrizedPassName(Name, NAME)) { \
605-
auto L = PassBuilder::parsePassParameters(parseLevelParam, Name, NAME); \
606-
if (!L) \
607-
return (errs() << NAME ": " << toString(L.takeError()) << '\n', false); \
616+
auto L = PassBuilder::parsePassParameters(parseOptLevelParam, Name, NAME); \
617+
if (!L) { \
618+
errs() << NAME ": " << toString(L.takeError()) << '\n'; \
619+
return false; \
620+
} \
608621
INVOKE(PM, L.get()); \
609622
return true; \
610623
}
@@ -615,13 +628,15 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
615628

616629
// Loop-level callbacks
617630
registerPipelineParsingCallback(
618-
[this, parseLevelParam](StringRef Name, LoopPassManager &PM,
619-
ArrayRef<PassBuilder::PipelineElement>) {
631+
[this](StringRef Name, LoopPassManager &PM,
632+
ArrayRef<PassBuilder::PipelineElement>) {
620633
#define LOOP_CALLBACK(NAME, INVOKE) \
621634
if (PassBuilder::checkParametrizedPassName(Name, NAME)) { \
622-
auto L = PassBuilder::parsePassParameters(parseLevelParam, Name, NAME); \
623-
if (!L) \
624-
return (errs() << NAME ": " << toString(L.takeError()) << '\n', false); \
635+
auto L = PassBuilder::parsePassParameters(parseOptLevelParam, Name, NAME); \
636+
if (!L) { \
637+
errs() << NAME ": " << toString(L.takeError()) << '\n'; \
638+
return false; \
639+
} \
625640
INVOKE(PM, L.get()); \
626641
return true; \
627642
}
@@ -714,25 +729,6 @@ static std::optional<int> parseDevirtPassName(StringRef Name) {
714729
return Count;
715730
}
716731

717-
static std::optional<OptimizationLevel> parseOptLevel(StringRef S) {
718-
return StringSwitch<std::optional<OptimizationLevel>>(S)
719-
.Case("O0", OptimizationLevel::O0)
720-
.Case("O1", OptimizationLevel::O1)
721-
.Case("O2", OptimizationLevel::O2)
722-
.Case("O3", OptimizationLevel::O3)
723-
.Case("Os", OptimizationLevel::Os)
724-
.Case("Oz", OptimizationLevel::Oz)
725-
.Default(std::nullopt);
726-
}
727-
728-
static Expected<OptimizationLevel> parseOptLevelParam(StringRef S) {
729-
std::optional<OptimizationLevel> OptLevel = parseOptLevel(S);
730-
if (OptLevel)
731-
return *OptLevel;
732-
return make_error<StringError>(
733-
formatv("invalid optimization level '{}'", S).str(),
734-
inconvertibleErrorCode());
735-
}
736732

737733
Expected<bool> PassBuilder::parseSinglePassOption(StringRef Params,
738734
StringRef OptionName,

‎llvm/test/Other/pipeline-callbacks-string-api.ll‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
; RUN: opt -disable-output -print-pipeline-passes \
32
; RUN: -passes-ep-vectorizer-start='no-op-function' \
43
; RUN: -passes='function(VectorizerStartCallbacks<O3>)' < %s 2>&1 | FileCheck %s --check-prefix=VECSTART
@@ -11,12 +10,40 @@
1110
; RUN: opt -disable-output -print-pipeline-passes \
1211
; RUN: -passes-ep-pipeline-early-simplification='no-op-module' \
1312
; RUN: -passes='PipelineEarlySimplificationCallbacks<O2>' < %s 2>&1 | FileCheck %s --check-prefix=LTOEARLY
13+
; RUN: opt -disable-output -print-pipeline-passes \
14+
; RUN: -passes-ep-optimizer-early='no-op-module' \
15+
; RUN: -passes='OptimizerEarlyCallbacks<O2>' < %s 2>&1 | FileCheck %s --check-prefix=OPTEARLY
16+
; RUN: opt -disable-output -print-pipeline-passes \
17+
; RUN: -passes-ep-optimizer-last='no-op-module' \
18+
; RUN: -passes='OptimizerLastCallbacks<O2>' < %s 2>&1 | FileCheck %s --check-prefix=OPTLAST
19+
; RUN: opt -disable-output -print-pipeline-passes \
20+
; RUN: -passes-ep-scalar-optimizer-late='no-op-function' \
21+
; RUN: -passes='function(ScalarOptimizerLateCallbacks<O2>)' < %s 2>&1 | FileCheck %s --check-prefix=SCALATE
22+
; RUN: opt -disable-output -print-pipeline-passes \
23+
; RUN: -passes-ep-vectorizer-end='no-op-function' \
24+
; RUN: -passes='function(VectorizerEndCallbacks<O3>)' < %s 2>&1 | FileCheck %s --check-prefix=VECEND
25+
; RUN: opt -disable-output -print-pipeline-passes \
26+
; RUN: -passes-ep-late-loop-optimizations='no-op-loop' \
27+
; RUN: -passes='loop(LateLoopOptimizationsCallbacks<O2>)' < %s 2>&1 | FileCheck %s --check-prefix=LATELOOP
28+
; RUN: opt -disable-output -print-pipeline-passes \
29+
; RUN: -passes-ep-loop-optimizer-end='no-op-loop' \
30+
; RUN: -passes='loop(LoopOptimizerEndCallbacks<O2>)' < %s 2>&1 | FileCheck %s --check-prefix=LOOPOPTEND
31+
; RUN: opt -disable-output -print-pipeline-passes \
32+
; RUN: -passes-ep-cgscc-optimizer-late='no-op-cgscc' \
33+
; RUN: -passes='cgscc(CGSCCOptimizerLateCallbacks<O2>)' < %s 2>&1 | FileCheck %s --check-prefix=CGSCCTLATE
1434
; RUN: not opt -disable-output -passes='VectorizerStartCallbacks<foo>' < %s 2>&1 | FileCheck %s --check-prefix=INVALID
1535

1636
; VECSTART: no-op-function
1737
; PEEP: no-op-function
1838
; MODSTART: no-op-module
1939
; LTOEARLY: no-op-module
40+
; OPTEARLY: no-op-module
41+
; OPTLAST: no-op-module
42+
; SCALATE: no-op-function
43+
; VECEND: no-op-function
44+
; LATELOOP: no-op-loop
45+
; LOOPOPTEND: no-op-loop
46+
; CGSCCTLATE: no-op-cgscc
2047
; INVALID: invalid optimization level 'foo'
2148

2249
define void @f() {

0 commit comments

Comments
 (0)