[RISCV][GISEL] Introduce the RISCVPostLegalizerLowering pass #108991
Conversation
This is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines. This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run. We will be adding combines to this pass in future patches.
michaelmaitland
requested review from
aemerson,
arsenm,
ornata,
tschuett,
davemgreen and
topperc
|
@llvm/pr-subscribers-llvm-globalisel Author: Michael Maitland (michaelmaitland) ChangesThis is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines. This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run. We will be adding combines to this pass in future patches. Full diff: https://github.com/llvm/llvm-project/pull/108991.diff 6 Files Affected:
diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
index 124bc239451aed..7d02d630a28199 100644
--- a/llvm/lib/Target/RISCV/CMakeLists.txt
+++ b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -24,6 +24,8 @@ tablegen(LLVM RISCVGenPreLegalizeGICombiner.inc -gen-global-isel-combiner
-combiners="RISCVPreLegalizerCombiner")
tablegen(LLVM RISCVGenPostLegalizeGICombiner.inc -gen-global-isel-combiner
-combiners="RISCVPostLegalizerCombiner")
+tablegen(LLVM RISCVGenPostLegalizeGILowering.inc -gen-global-isel-combiner
+ -combiners="RISCVPostLegalizerLowering")
add_public_tablegen_target(RISCVCommonTableGen)
@@ -63,6 +65,7 @@ add_llvm_target(RISCVCodeGen
GISel/RISCVInstructionSelector.cpp
GISel/RISCVLegalizerInfo.cpp
GISel/RISCVPostLegalizerCombiner.cpp
+ GISel/RISCVPostLegalizerLowering.cpp
GISel/RISCVO0PreLegalizerCombiner.cpp
GISel/RISCVPreLegalizerCombiner.cpp
GISel/RISCVRegisterBankInfo.cpp
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp
new file mode 100644
index 00000000000000..66db15e3a2e28c
--- /dev/null
+++ b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp
@@ -0,0 +1,154 @@
+//===--------------- RISCVPostLegalizerLowering.cpp -------------*- 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
+/// Post-legalization lowering for instructions.
+///
+/// This is used to offload pattern matching from the selector.
+///
+/// General optimization combines should be handled by either the
+/// RISCVPostLegalizerCombiner or the RISCVPreLegalizerCombiner.
+///
+//===----------------------------------------------------------------------===//
+
+#include "RISCVSubtarget.h"
+
+#include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Debug.h"
+
+#define GET_GICOMBINER_DEPS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_DEPS
+
+#define DEBUG_TYPE "riscv-postlegalizer-lowering"
+
+using namespace llvm;
+
+namespace {
+
+#define GET_GICOMBINER_TYPES
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_TYPES
+
+class RISCVPostLegalizerLoweringImpl : public Combiner {
+protected:
+ // TODO: Make CombinerHelper methods const.
+ mutable CombinerHelper Helper;
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig;
+ const RISCVSubtarget &STI;
+
+public:
+ RISCVPostLegalizerLoweringImpl(
+ MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+ GISelCSEInfo *CSEInfo,
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig,
+ const RISCVSubtarget &STI);
+
+ static const char *getName() { return "RISCVPreLegalizerCombiner"; }
+
+ bool tryCombineAll(MachineInstr &I) const override;
+
+private:
+#define GET_GICOMBINER_CLASS_MEMBERS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_CLASS_MEMBERS
+};
+
+#define GET_GICOMBINER_IMPL
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_IMPL
+
+RISCVPostLegalizerLoweringImpl::RISCVPostLegalizerLoweringImpl(
+ MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+ GISelCSEInfo *CSEInfo,
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig,
+ const RISCVSubtarget &STI)
+ : Combiner(MF, CInfo, TPC, /*KB*/ nullptr, CSEInfo),
+ Helper(Observer, B, /*IsPreLegalize*/ true), RuleConfig(RuleConfig),
+ STI(STI),
+#define GET_GICOMBINER_CONSTRUCTOR_INITS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_CONSTRUCTOR_INITS
+{
+}
+
+class RISCVPostLegalizerLowering : public MachineFunctionPass {
+public:
+ static char ID;
+
+ RISCVPostLegalizerLowering();
+
+ StringRef getPassName() const override {
+ return "RISCVPostLegalizerLowering";
+ }
+
+ bool runOnMachineFunction(MachineFunction &MF) override;
+ void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+private:
+ RISCVPostLegalizerLoweringImplRuleConfig RuleConfig;
+};
+} // end anonymous namespace
+
+void RISCVPostLegalizerLowering::getAnalysisUsage(AnalysisUsage &AU) const {
+ AU.addRequired<TargetPassConfig>();
+ AU.setPreservesCFG();
+ getSelectionDAGFallbackAnalysisUsage(AU);
+ MachineFunctionPass::getAnalysisUsage(AU);
+}
+
+RISCVPostLegalizerLowering::RISCVPostLegalizerLowering()
+ : MachineFunctionPass(ID) {
+ if (!RuleConfig.parseCommandLineOption())
+ report_fatal_error("Invalid rule identifier");
+}
+
+bool RISCVPostLegalizerLowering::runOnMachineFunction(MachineFunction &MF) {
+ if (MF.getProperties().hasProperty(
+ MachineFunctionProperties::Property::FailedISel))
+ return false;
+ assert(MF.getProperties().hasProperty(
+ MachineFunctionProperties::Property::Legalized) &&
+ "Expected a legalized function?");
+ auto *TPC = &getAnalysis<TargetPassConfig>();
+ const Function &F = MF.getFunction();
+
+ const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
+ CombinerInfo CInfo(/*AllowIllegalOps*/ true, /*ShouldLegalizeIllegal*/ false,
+ /*LegalizerInfo*/ nullptr, /*OptEnabled=*/true,
+ F.hasOptSize(), F.hasMinSize());
+ // Disable fixed-point iteration to reduce compile-time
+ CInfo.MaxIterations = 1;
+ CInfo.ObserverLvl = CombinerInfo::ObserverLevel::SinglePass;
+ // PostLegalizerCombiner performs DCE, so a full DCE pass is unnecessary.
+ CInfo.EnableFullDCE = false;
+ RISCVPostLegalizerLoweringImpl Impl(MF, CInfo, TPC, /*CSEInfo*/ nullptr,
+ RuleConfig, ST);
+ return Impl.combineMachineInstrs();
+}
+
+char RISCVPostLegalizerLowering::ID = 0;
+INITIALIZE_PASS_BEGIN(RISCVPostLegalizerLowering, DEBUG_TYPE,
+ "Lower RISC-V MachineInstrs after legalization", false,
+ false)
+INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
+INITIALIZE_PASS_END(RISCVPostLegalizerLowering, DEBUG_TYPE,
+ "Lower RISC-V MachineInstrs after legalization", false,
+ false)
+
+namespace llvm {
+FunctionPass *createRISCVPostLegalizerLowering() {
+ return new RISCVPostLegalizerLowering();
+}
+} // end namespace llvm
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
index 5a94ada8f8dd46..561e4895e19428 100644
--- a/llvm/lib/Target/RISCV/RISCV.h
+++ b/llvm/lib/Target/RISCV/RISCV.h
@@ -99,6 +99,9 @@ void initializeRISCVO0PreLegalizerCombinerPass(PassRegistry &);
FunctionPass *createRISCVPreLegalizerCombiner();
void initializeRISCVPreLegalizerCombinerPass(PassRegistry &);
+
+FunctionPass *createRISCVPostLegalizerLowering();
+void initializeRISCVPostLegalizerLoweringPass(PassRegistry &);
} // namespace llvm
#endif
diff --git a/llvm/lib/Target/RISCV/RISCVCombine.td b/llvm/lib/Target/RISCV/RISCVCombine.td
index 3a5afb1b075c9e..d48698ae6f2bfb 100644
--- a/llvm/lib/Target/RISCV/RISCVCombine.td
+++ b/llvm/lib/Target/RISCV/RISCVCombine.td
@@ -19,6 +19,13 @@ def RISCVO0PreLegalizerCombiner: GICombiner<
"RISCVO0PreLegalizerCombinerImpl", [optnone_combines]> {
}
+// Post-legalization combines which should happen at all optimization levels.
+// (E.g. ones that facilitate matching for the selector) For example, matching
+// pseudos.
+def RISCVPostLegalizerLowering
+ : GICombiner<"RISCVPostLegalizerLoweringImpl", []> {
+}
+
// Post-legalization combines which are primarily optimizations.
// TODO: Add more combines.
def RISCVPostLegalizerCombiner
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 794df2212dfa53..5cb9fbc8a9eb98 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -111,6 +111,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
initializeRISCVO0PreLegalizerCombinerPass(*PR);
initializeRISCVPreLegalizerCombinerPass(*PR);
initializeRISCVPostLegalizerCombinerPass(*PR);
+ initializeRISCVPostLegalizerLoweringPass(*PR);
initializeKCFIPass(*PR);
initializeRISCVDeadRegisterDefinitionsPass(*PR);
initializeRISCVMakeCompressibleOptPass(*PR);
@@ -482,6 +483,7 @@ bool RISCVPassConfig::addLegalizeMachineIR() {
void RISCVPassConfig::addPreRegBankSelect() {
if (getOptLevel() != CodeGenOptLevel::None)
addPass(createRISCVPostLegalizerCombiner());
+ addPass(createRISCVPostLegalizerLowering());
}
bool RISCVPassConfig::addRegBankSelect() {
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
index 82c66c23b26ba9..75607bb81ff513 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
@@ -23,6 +23,7 @@
; ENABLED-NEXT: Legalizer
; ENABLED-O1-NEXT: MachineDominator Tree Construction
; ENABLED-O1-NEXT: RISCVPostLegalizerCombiner
+; ENABLED-NEXT: RISCVPostLegalizerLowering
; ENABLED-NEXT: RegBankSelect
; ENABLED-NEXT: Analysis for ComputingKnownBits
; ENABLED-O1-NEXT: Lazy Branch Probability Analysis
|
|
@llvm/pr-subscribers-backend-risc-v Author: Michael Maitland (michaelmaitland) ChangesThis is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines. This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run. We will be adding combines to this pass in future patches. Full diff: https://github.com/llvm/llvm-project/pull/108991.diff 6 Files Affected:
diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
index 124bc239451aed..7d02d630a28199 100644
--- a/llvm/lib/Target/RISCV/CMakeLists.txt
+++ b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -24,6 +24,8 @@ tablegen(LLVM RISCVGenPreLegalizeGICombiner.inc -gen-global-isel-combiner
-combiners="RISCVPreLegalizerCombiner")
tablegen(LLVM RISCVGenPostLegalizeGICombiner.inc -gen-global-isel-combiner
-combiners="RISCVPostLegalizerCombiner")
+tablegen(LLVM RISCVGenPostLegalizeGILowering.inc -gen-global-isel-combiner
+ -combiners="RISCVPostLegalizerLowering")
add_public_tablegen_target(RISCVCommonTableGen)
@@ -63,6 +65,7 @@ add_llvm_target(RISCVCodeGen
GISel/RISCVInstructionSelector.cpp
GISel/RISCVLegalizerInfo.cpp
GISel/RISCVPostLegalizerCombiner.cpp
+ GISel/RISCVPostLegalizerLowering.cpp
GISel/RISCVO0PreLegalizerCombiner.cpp
GISel/RISCVPreLegalizerCombiner.cpp
GISel/RISCVRegisterBankInfo.cpp
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp
new file mode 100644
index 00000000000000..66db15e3a2e28c
--- /dev/null
+++ b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerLowering.cpp
@@ -0,0 +1,154 @@
+//===--------------- RISCVPostLegalizerLowering.cpp -------------*- 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
+/// Post-legalization lowering for instructions.
+///
+/// This is used to offload pattern matching from the selector.
+///
+/// General optimization combines should be handled by either the
+/// RISCVPostLegalizerCombiner or the RISCVPreLegalizerCombiner.
+///
+//===----------------------------------------------------------------------===//
+
+#include "RISCVSubtarget.h"
+
+#include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Debug.h"
+
+#define GET_GICOMBINER_DEPS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_DEPS
+
+#define DEBUG_TYPE "riscv-postlegalizer-lowering"
+
+using namespace llvm;
+
+namespace {
+
+#define GET_GICOMBINER_TYPES
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_TYPES
+
+class RISCVPostLegalizerLoweringImpl : public Combiner {
+protected:
+ // TODO: Make CombinerHelper methods const.
+ mutable CombinerHelper Helper;
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig;
+ const RISCVSubtarget &STI;
+
+public:
+ RISCVPostLegalizerLoweringImpl(
+ MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+ GISelCSEInfo *CSEInfo,
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig,
+ const RISCVSubtarget &STI);
+
+ static const char *getName() { return "RISCVPreLegalizerCombiner"; }
+
+ bool tryCombineAll(MachineInstr &I) const override;
+
+private:
+#define GET_GICOMBINER_CLASS_MEMBERS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_CLASS_MEMBERS
+};
+
+#define GET_GICOMBINER_IMPL
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_IMPL
+
+RISCVPostLegalizerLoweringImpl::RISCVPostLegalizerLoweringImpl(
+ MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+ GISelCSEInfo *CSEInfo,
+ const RISCVPostLegalizerLoweringImplRuleConfig &RuleConfig,
+ const RISCVSubtarget &STI)
+ : Combiner(MF, CInfo, TPC, /*KB*/ nullptr, CSEInfo),
+ Helper(Observer, B, /*IsPreLegalize*/ true), RuleConfig(RuleConfig),
+ STI(STI),
+#define GET_GICOMBINER_CONSTRUCTOR_INITS
+#include "RISCVGenPostLegalizeGILowering.inc"
+#undef GET_GICOMBINER_CONSTRUCTOR_INITS
+{
+}
+
+class RISCVPostLegalizerLowering : public MachineFunctionPass {
+public:
+ static char ID;
+
+ RISCVPostLegalizerLowering();
+
+ StringRef getPassName() const override {
+ return "RISCVPostLegalizerLowering";
+ }
+
+ bool runOnMachineFunction(MachineFunction &MF) override;
+ void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+private:
+ RISCVPostLegalizerLoweringImplRuleConfig RuleConfig;
+};
+} // end anonymous namespace
+
+void RISCVPostLegalizerLowering::getAnalysisUsage(AnalysisUsage &AU) const {
+ AU.addRequired<TargetPassConfig>();
+ AU.setPreservesCFG();
+ getSelectionDAGFallbackAnalysisUsage(AU);
+ MachineFunctionPass::getAnalysisUsage(AU);
+}
+
+RISCVPostLegalizerLowering::RISCVPostLegalizerLowering()
+ : MachineFunctionPass(ID) {
+ if (!RuleConfig.parseCommandLineOption())
+ report_fatal_error("Invalid rule identifier");
+}
+
+bool RISCVPostLegalizerLowering::runOnMachineFunction(MachineFunction &MF) {
+ if (MF.getProperties().hasProperty(
+ MachineFunctionProperties::Property::FailedISel))
+ return false;
+ assert(MF.getProperties().hasProperty(
+ MachineFunctionProperties::Property::Legalized) &&
+ "Expected a legalized function?");
+ auto *TPC = &getAnalysis<TargetPassConfig>();
+ const Function &F = MF.getFunction();
+
+ const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
+ CombinerInfo CInfo(/*AllowIllegalOps*/ true, /*ShouldLegalizeIllegal*/ false,
+ /*LegalizerInfo*/ nullptr, /*OptEnabled=*/true,
+ F.hasOptSize(), F.hasMinSize());
+ // Disable fixed-point iteration to reduce compile-time
+ CInfo.MaxIterations = 1;
+ CInfo.ObserverLvl = CombinerInfo::ObserverLevel::SinglePass;
+ // PostLegalizerCombiner performs DCE, so a full DCE pass is unnecessary.
+ CInfo.EnableFullDCE = false;
+ RISCVPostLegalizerLoweringImpl Impl(MF, CInfo, TPC, /*CSEInfo*/ nullptr,
+ RuleConfig, ST);
+ return Impl.combineMachineInstrs();
+}
+
+char RISCVPostLegalizerLowering::ID = 0;
+INITIALIZE_PASS_BEGIN(RISCVPostLegalizerLowering, DEBUG_TYPE,
+ "Lower RISC-V MachineInstrs after legalization", false,
+ false)
+INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
+INITIALIZE_PASS_END(RISCVPostLegalizerLowering, DEBUG_TYPE,
+ "Lower RISC-V MachineInstrs after legalization", false,
+ false)
+
+namespace llvm {
+FunctionPass *createRISCVPostLegalizerLowering() {
+ return new RISCVPostLegalizerLowering();
+}
+} // end namespace llvm
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
index 5a94ada8f8dd46..561e4895e19428 100644
--- a/llvm/lib/Target/RISCV/RISCV.h
+++ b/llvm/lib/Target/RISCV/RISCV.h
@@ -99,6 +99,9 @@ void initializeRISCVO0PreLegalizerCombinerPass(PassRegistry &);
FunctionPass *createRISCVPreLegalizerCombiner();
void initializeRISCVPreLegalizerCombinerPass(PassRegistry &);
+
+FunctionPass *createRISCVPostLegalizerLowering();
+void initializeRISCVPostLegalizerLoweringPass(PassRegistry &);
} // namespace llvm
#endif
diff --git a/llvm/lib/Target/RISCV/RISCVCombine.td b/llvm/lib/Target/RISCV/RISCVCombine.td
index 3a5afb1b075c9e..d48698ae6f2bfb 100644
--- a/llvm/lib/Target/RISCV/RISCVCombine.td
+++ b/llvm/lib/Target/RISCV/RISCVCombine.td
@@ -19,6 +19,13 @@ def RISCVO0PreLegalizerCombiner: GICombiner<
"RISCVO0PreLegalizerCombinerImpl", [optnone_combines]> {
}
+// Post-legalization combines which should happen at all optimization levels.
+// (E.g. ones that facilitate matching for the selector) For example, matching
+// pseudos.
+def RISCVPostLegalizerLowering
+ : GICombiner<"RISCVPostLegalizerLoweringImpl", []> {
+}
+
// Post-legalization combines which are primarily optimizations.
// TODO: Add more combines.
def RISCVPostLegalizerCombiner
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 794df2212dfa53..5cb9fbc8a9eb98 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -111,6 +111,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
initializeRISCVO0PreLegalizerCombinerPass(*PR);
initializeRISCVPreLegalizerCombinerPass(*PR);
initializeRISCVPostLegalizerCombinerPass(*PR);
+ initializeRISCVPostLegalizerLoweringPass(*PR);
initializeKCFIPass(*PR);
initializeRISCVDeadRegisterDefinitionsPass(*PR);
initializeRISCVMakeCompressibleOptPass(*PR);
@@ -482,6 +483,7 @@ bool RISCVPassConfig::addLegalizeMachineIR() {
void RISCVPassConfig::addPreRegBankSelect() {
if (getOptLevel() != CodeGenOptLevel::None)
addPass(createRISCVPostLegalizerCombiner());
+ addPass(createRISCVPostLegalizerLowering());
}
bool RISCVPassConfig::addRegBankSelect() {
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
index 82c66c23b26ba9..75607bb81ff513 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
@@ -23,6 +23,7 @@
; ENABLED-NEXT: Legalizer
; ENABLED-O1-NEXT: MachineDominator Tree Construction
; ENABLED-O1-NEXT: RISCVPostLegalizerCombiner
+; ENABLED-NEXT: RISCVPostLegalizerLowering
; ENABLED-NEXT: RegBankSelect
; ENABLED-NEXT: Analysis for ComputingKnownBits
; ENABLED-O1-NEXT: Lazy Branch Probability Analysis
|
|
Are you sure you need this? @tobias-stadler in #97670 allowed us to re-select generic instructions in the selector, which I think should allow us to avoid having a lower phase. |
We took this because we wanted to avoid introducing target specific generic opcodes in the legalizer so that the post legalization combiner would only have to consider non-target specific generic opcodes. We still need to lower before instruction selection to target specific generic opcodes. We thought that this was the place to do it. It looks like thats what AArch64 does. In order to understand whether we need this, I'd first like to ask two questions:
|
…8991) This is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines. This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run. We will be adding combines to this pass in future patches.
|
Currently, AArch64PostLegalizerLowering is needed and not deprecated, but I have started to reevaluate its necessity. Overall, I think it would be best to refrain from using Combiners for lowering. We probably need some more infrastructure work, before we can decide what's the best approach for these kinds of transformations though. I wanted to start a discussion on this when I have more time, but I can preliminarily dump some of my thoughts here. As I see it, AArch64PostLegalizerLowering currently contains 2 classes of combines:
In my opinion, class 1 combines can and should be treated by reselecting generic instructions in the InstructionSelector, because they are only one-shot transformations, so they don't need to and therefore shouldn't happen inside a combiner. Instead these transformations can e.g. be done by creating generic instructions and recursively calling select() on them (which is only legal in all cases since #97670). I am thinking about adding a local worklist to InstructionSelect, so that instead of recursively calling select(), we can just mark instructions for reselection and InstructionSelect takes care of the rest. I have draft patches for this, but I am not sure this is the way to go, because it is only marginally more convenient than recursively calling select(). The problem is that this doesn't work for class 2 of combines, because they need to be fully applied to the entire MIR before ISel runs otherwise this can cause missed folding opportunities. They therefore need to happen inside a combiner. A solution for this would be to just throw these inside PostLegalizerCombiner and live with some O0 code-size regressions (haven't measured impact yet). Alternatively, once we have gMIR TableGen pattern support and ported enough ad-hoc matching code to TableGen, we can let TableGen deal with commutative instructions automagically and remove those combines entirely. With my recent improvements to the core Combiner algorithm, AArch64PostLegalizerLowering went from ~4.5% to ~2.4% O0 and from ~1.1% to ~0.6% O2 back-end time, so this pass isn't as eye-wateringly expensive as it used to be, but still pretty expensive. (Other O2 times for reference: AArch64PreLegalizerCombiner 4.1%, AArch64PostLegalizerCombiner 1.9%, InstructionSelect: 3.4%). Once we have gMIR TableGen ISel patterns, porting combines to and from ISel should become very easy, so adding PostLegalizerLowering right now might not be difficult to reverse mistake. |
|
@tobias-stadler Thank you for the insights. I took the advice and began to port #108859 to the instruction selector. I quickly ran into the problem that G_* instructions created in the instruction selector are not regbankselected. If you check out #108859, you can see that I started to constrain them, but apparently only !preISelGenericOpcode instructions are allowed to be constrained. So I am not sure how to constrain them, even if I add in all the extra code to do it. Also, it seems like I cannot test the intermediate lowering since whatever we lower to gets selected and that is what gets tested. This seems against the core value of GISEL easy ability to test specific things. I am wondering if you have run into this problem and what your thoughts are on it. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/14/builds/1433 Here is the relevant piece of the build log for the reference |
Last time I looked at AArch64PostLegalizerLowering almost all lowering combines were fairly trivial (mostly single instruction rewrites), so I didn't think these technicalities would be an issue. Your lowering appears to be more complex, so I completely agree.
I don't think I quite understand this issue. It sounds like you were trying to constrain a RegClass? You should be able to at least use These technicalities seem solvable though, some ideas off the top of my head:
I wanted to experiment with these things before I ramble about this without proposing a proper solution. It probably makes the most sense to just run with RISCVPostLegalizerLowering for now unless you feel like experimenting or can live with these limitations. I am not really attached to the idea of reselecting generic instructions (I hope it will work for AArch64 though). I just think that Combiners are the wrong place for one-shot expansions due to high overhead. In these cases SelectionDAG's split Type and Operation legalization phases kinda make sense. It could make sense to create stripped down single-pass low-overhead versions of either the Legalizer or the Combiner (e.g. without DCE, WorkList, ...) for these cases, but the problem is that just walking over the instructions and searching for certain opcodes is a significant compile-time contributor for O0, so we should aim to reduce the number of necessary O0 passes. |
|
One observation: If you create G_X in the lowering pass, you have to regbank-select them, but we have a pass which is an expert for it. In this case, it is probably better to do the lowering in the legalizer. As a by-product, we get legal G_X. |
regbank select runs after the lowering pass... |
|
It leaves the question of legal G_X. |
|
Ok I concede we'll do it in the legalizer. |
|
Could you tag me when you encounter other cases that are annoying to solve using Legalizer? I'd like to compile these examples, so we can eventually find a better solution. |
|
My mistake, I thought all the pieces to deprecate this for AArch64 was there. In any case, I want to give some more historical context for why we had this at all. Basically we wanted to do rewrites late once all optimizations were done (including all the post-legalizer combines). We would have done it in the selector if we could at the time. As Tobias said these are simple since they're basically selection split into two parts. I would recommend the following for RISCV:
|
…8991) This is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines. This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run. We will be adding combines to this pass in future patches.
…108991)" This reverts commit 6497283. Based on the discussions in #108991 that happened post merge, we have decided to remove this pass in favor of generating `RISCV::G_*` opcodes in the legalizer. We may reconsider moving that code elsewhere in the future so that we can do a better job during generic combines. We don't feel that doing it in instruciton selection is the right decision today. Firstly, it requires us to manually do regbankselect on the newly introduced instructions. Secondly, it is more difficult to test since the test output will contain whatever `RISCV::G_*` instructions select to (instead of `RISCV::G_*`). My personal opinion is that the legalizer pass can be split into an early legalizer and a late legalizer, both before regbankselect. The first legalizer would not introduce target specific generic opcodes and the generic combiner would run after it. The second legalizer would introduce the target specific generic opcodes. I think this approach is better than the lowerer because the legalizer guarantees that whatever we lower to is legal, and apparently because it is more performant at compared to the lowerer (although, I'm not sure how true this is).
This is mostly a copy of the AArch64PostLegalizerLoweringPass, except it removes all of the AArch64 combines.
This pass allows us to lower instructions after the generic post-legalization combiner has had a chance to run.
We will be adding combines to this pass in future patches.