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.

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

[RISCV][GISel] Add a post legalizer combiner and enable a couple comb… #67053

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Sep 21, 2023

…ines.

We have an existing test that shows benefit from redundant_and and identity combines so use them as a starting point.

…ines.

We have an existing test that shows benefit from redundant_and and
identity combines so use them as a starting point.
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 21, 2023

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-risc-v

Changes

…ines.

We have an existing test that shows benefit from redundant_and and identity combines so use them as a starting point.


Full diff: https://github.com/llvm/llvm-project/pull/67053.diff

9 Files Affected:

  • (modified) llvm/lib/Target/RISCV/CMakeLists.txt (+3)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVO0PreLegalizerCombiner.cpp (+2-2)
  • (added) llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp (+173)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp (+2-2)
  • (modified) llvm/lib/Target/RISCV/RISCV.h (+3)
  • (modified) llvm/lib/Target/RISCV/RISCVCombine.td (+7)
  • (modified) llvm/lib/Target/RISCV/RISCVTargetMachine.cpp (+7)
  • (modified) llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll (+1-9)
  • (modified) llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll (+2)
diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
index abe2f5a6246fc25..fd5a5244486ab18 100644
--- a/llvm/lib/Target/RISCV/CMakeLists.txt
+++ b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -21,6 +21,8 @@ tablegen(LLVM RISCVGenO0PreLegalizeGICombiner.inc -gen-global-isel-combiner
               -combiners="RISCVO0PreLegalizerCombiner")
 tablegen(LLVM RISCVGenPreLegalizeGICombiner.inc -gen-global-isel-combiner
               -combiners="RISCVPreLegalizerCombiner")
+tablegen(LLVM RISCVGenPostLegalizeGICombiner.inc -gen-global-isel-combiner
+              -combiners="RISCVPostLegalizerCombiner")
 
 add_public_tablegen_target(RISCVCommonTableGen)
 
@@ -54,6 +56,7 @@ add_llvm_target(RISCVCodeGen
   GISel/RISCVCallLowering.cpp
   GISel/RISCVInstructionSelector.cpp
   GISel/RISCVLegalizerInfo.cpp
+  GISel/RISCVPostLegalizerCombiner.cpp
   GISel/RISCVO0PreLegalizerCombiner.cpp
   GISel/RISCVPreLegalizerCombiner.cpp
   GISel/RISCVRegisterBankInfo.cpp
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVO0PreLegalizerCombiner.cpp b/llvm/lib/Target/RISCV/GISel/RISCVO0PreLegalizerCombiner.cpp
index d94edbc402e6156..be77979512e00c5 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVO0PreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVO0PreLegalizerCombiner.cpp
@@ -139,13 +139,13 @@ bool RISCVO0PreLegalizerCombiner::runOnMachineFunction(MachineFunction &MF) {
 
 char RISCVO0PreLegalizerCombiner::ID = 0;
 INITIALIZE_PASS_BEGIN(RISCVO0PreLegalizerCombiner, DEBUG_TYPE,
-                      "Combine RISCV machine instrs before legalization", false,
+                      "Combine RISC-V machine instrs before legalization", false,
                       false)
 INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
 INITIALIZE_PASS_DEPENDENCY(GISelKnownBitsAnalysis)
 INITIALIZE_PASS_DEPENDENCY(GISelCSEAnalysisWrapperPass)
 INITIALIZE_PASS_END(RISCVO0PreLegalizerCombiner, DEBUG_TYPE,
-                    "Combine RISCV machine instrs before legalization", false,
+                    "Combine RISC-V machine instrs before legalization", false,
                     false)
 
 namespace llvm {
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp
new file mode 100644
index 000000000000000..9c28944abc76721
--- /dev/null
+++ b/llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp
@@ -0,0 +1,173 @@
+//=== RISCVPostLegalizerCombiner.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 combines on generic MachineInstrs.
+///
+/// The combines here must preserve instruction legality.
+///
+/// Combines which don't rely on instruction legality should go in the
+/// RISCVPreLegalizerCombiner.
+///
+//===----------------------------------------------------------------------===//
+
+#include "RISCVTargetMachine.h"
+#include "llvm/CodeGen/GlobalISel/CSEInfo.h"
+#include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/CombinerInfo.h"
+#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h"
+#include "llvm/CodeGen/GlobalISel/GISelKnownBits.h"
+#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/MachineDominators.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
+
+#define GET_GICOMBINER_DEPS
+#include "RISCVGenPostLegalizeGICombiner.inc"
+#undef GET_GICOMBINER_DEPS
+
+#define DEBUG_TYPE "riscv-postlegalizer-combiner"
+
+using namespace llvm;
+
+namespace {
+
+#define GET_GICOMBINER_TYPES
+#include "RISCVGenPostLegalizeGICombiner.inc"
+#undef GET_GICOMBINER_TYPES
+
+class RISCVPostLegalizerCombinerImpl : public Combiner {
+protected:
+  // TODO: Make CombinerHelper methods const.
+  mutable CombinerHelper Helper;
+  const RISCVPostLegalizerCombinerImplRuleConfig &RuleConfig;
+  const RISCVSubtarget &STI;
+
+public:
+  RISCVPostLegalizerCombinerImpl(
+      MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+      GISelKnownBits &KB, GISelCSEInfo *CSEInfo,
+      const RISCVPostLegalizerCombinerImplRuleConfig &RuleConfig,
+      const RISCVSubtarget &STI, MachineDominatorTree *MDT,
+      const LegalizerInfo *LI);
+
+  static const char *getName() { return "RISCVPostLegalizerCombiner"; }
+
+  bool tryCombineAll(MachineInstr &I) const override;
+
+private:
+#define GET_GICOMBINER_CLASS_MEMBERS
+#include "RISCVGenPostLegalizeGICombiner.inc"
+#undef GET_GICOMBINER_CLASS_MEMBERS
+};
+
+#define GET_GICOMBINER_IMPL
+#include "RISCVGenPostLegalizeGICombiner.inc"
+#undef GET_GICOMBINER_IMPL
+
+RISCVPostLegalizerCombinerImpl::RISCVPostLegalizerCombinerImpl(
+    MachineFunction &MF, CombinerInfo &CInfo, const TargetPassConfig *TPC,
+    GISelKnownBits &KB, GISelCSEInfo *CSEInfo,
+    const RISCVPostLegalizerCombinerImplRuleConfig &RuleConfig,
+    const RISCVSubtarget &STI, MachineDominatorTree *MDT,
+    const LegalizerInfo *LI)
+    : Combiner(MF, CInfo, TPC, &KB, CSEInfo),
+      Helper(Observer, B, /*IsPreLegalize*/ false, &KB, MDT, LI),
+      RuleConfig(RuleConfig), STI(STI),
+#define GET_GICOMBINER_CONSTRUCTOR_INITS
+#include "RISCVGenPostLegalizeGICombiner.inc"
+#undef GET_GICOMBINER_CONSTRUCTOR_INITS
+{
+}
+
+class RISCVPostLegalizerCombiner : public MachineFunctionPass {
+public:
+  static char ID;
+
+  RISCVPostLegalizerCombiner();
+
+  StringRef getPassName() const override {
+    return "RISCVPostLegalizerCombiner";
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+private:
+  RISCVPostLegalizerCombinerImplRuleConfig RuleConfig;
+};
+} // end anonymous namespace
+
+void RISCVPostLegalizerCombiner::getAnalysisUsage(AnalysisUsage &AU) const {
+  AU.addRequired<TargetPassConfig>();
+  AU.setPreservesCFG();
+  getSelectionDAGFallbackAnalysisUsage(AU);
+  AU.addRequired<GISelKnownBitsAnalysis>();
+  AU.addPreserved<GISelKnownBitsAnalysis>();
+  AU.addRequired<MachineDominatorTree>();
+  AU.addPreserved<MachineDominatorTree>();
+  AU.addRequired<GISelCSEAnalysisWrapperPass>();
+  AU.addPreserved<GISelCSEAnalysisWrapperPass>();
+  MachineFunctionPass::getAnalysisUsage(AU);
+}
+
+RISCVPostLegalizerCombiner::RISCVPostLegalizerCombiner()
+    : MachineFunctionPass(ID) {
+  initializeRISCVPostLegalizerCombinerPass(*PassRegistry::getPassRegistry());
+
+  if (!RuleConfig.parseCommandLineOption())
+    report_fatal_error("Invalid rule identifier");
+}
+
+bool RISCVPostLegalizerCombiner::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();
+  bool EnableOpt =
+      MF.getTarget().getOptLevel() != CodeGenOptLevel::None && !skipFunction(F);
+
+  const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
+  const auto *LI = ST.getLegalizerInfo();
+
+  GISelKnownBits *KB = &getAnalysis<GISelKnownBitsAnalysis>().get(MF);
+  MachineDominatorTree *MDT = &getAnalysis<MachineDominatorTree>();
+  GISelCSEAnalysisWrapper &Wrapper =
+      getAnalysis<GISelCSEAnalysisWrapperPass>().getCSEWrapper();
+  auto *CSEInfo = &Wrapper.get(TPC->getCSEConfig());
+
+  CombinerInfo CInfo(/*AllowIllegalOps*/ true, /*ShouldLegalizeIllegal*/ false,
+                     /*LegalizerInfo*/ nullptr, EnableOpt, F.hasOptSize(),
+                     F.hasMinSize());
+  RISCVPostLegalizerCombinerImpl Impl(MF, CInfo, TPC, *KB, CSEInfo,
+                                        RuleConfig, ST, MDT, LI);
+  return Impl.combineMachineInstrs();
+}
+
+char RISCVPostLegalizerCombiner::ID = 0;
+INITIALIZE_PASS_BEGIN(RISCVPostLegalizerCombiner, DEBUG_TYPE,
+                      "Combine RISC-V MachineInstrs after legalization", false,
+                      false)
+INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
+INITIALIZE_PASS_DEPENDENCY(GISelKnownBitsAnalysis)
+INITIALIZE_PASS_END(RISCVPostLegalizerCombiner, DEBUG_TYPE,
+                    "Combine RISC-V MachineInstrs after legalization", false,
+                    false)
+
+namespace llvm {
+FunctionPass *createRISCVPostLegalizerCombiner() {
+  return new RISCVPostLegalizerCombiner();
+}
+} // end namespace llvm
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp b/llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp
index 36f5d957832218c..9a35fffae05890a 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp
@@ -153,13 +153,13 @@ bool RISCVPreLegalizerCombiner::runOnMachineFunction(MachineFunction &MF) {
 
 char RISCVPreLegalizerCombiner::ID = 0;
 INITIALIZE_PASS_BEGIN(RISCVPreLegalizerCombiner, DEBUG_TYPE,
-                      "Combine RISCV machine instrs before legalization", false,
+                      "Combine RISC-V machine instrs before legalization", false,
                       false)
 INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
 INITIALIZE_PASS_DEPENDENCY(GISelKnownBitsAnalysis)
 INITIALIZE_PASS_DEPENDENCY(GISelCSEAnalysisWrapperPass)
 INITIALIZE_PASS_END(RISCVPreLegalizerCombiner, DEBUG_TYPE,
-                    "Combine RISCV machine instrs before legalization", false,
+                    "Combine RISC-V machine instrs before legalization", false,
                     false)
 
 namespace llvm {
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
index 702f69bcd4780a9..0efc915ea52c550 100644
--- a/llvm/lib/Target/RISCV/RISCV.h
+++ b/llvm/lib/Target/RISCV/RISCV.h
@@ -84,6 +84,9 @@ InstructionSelector *createRISCVInstructionSelector(const RISCVTargetMachine &,
                                                     RISCVRegisterBankInfo &);
 void initializeRISCVDAGToDAGISelPass(PassRegistry &);
 
+FunctionPass *createRISCVPostLegalizerCombiner();
+void initializeRISCVPostLegalizerCombinerPass(PassRegistry &);
+
 FunctionPass *createRISCVO0PreLegalizerCombiner();
 void initializeRISCVO0PreLegalizerCombinerPass(PassRegistry &);
 
diff --git a/llvm/lib/Target/RISCV/RISCVCombine.td b/llvm/lib/Target/RISCV/RISCVCombine.td
index b632a4f54f202c9..3a5afb1b075c9e2 100644
--- a/llvm/lib/Target/RISCV/RISCVCombine.td
+++ b/llvm/lib/Target/RISCV/RISCVCombine.td
@@ -18,3 +18,10 @@ def RISCVPreLegalizerCombiner: GICombiner<
 def RISCVO0PreLegalizerCombiner: GICombiner<
   "RISCVO0PreLegalizerCombinerImpl", [optnone_combines]> {
 }
+
+// Post-legalization combines which are primarily optimizations.
+// TODO: Add more combines.
+def RISCVPostLegalizerCombiner
+    : GICombiner<"RISCVPostLegalizerCombinerImpl",
+                 [redundant_and, identity_combines]> {
+}
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 30d559a4958c320..69a0569fccc4eca 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -85,6 +85,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
   initializeGlobalISel(*PR);
   initializeRISCVO0PreLegalizerCombinerPass(*PR);
   initializeRISCVPreLegalizerCombinerPass(*PR);
+  initializeRISCVPostLegalizerCombinerPass(*PR);
   initializeKCFIPass(*PR);
   initializeRISCVDeadRegisterDefinitionsPass(*PR);
   initializeRISCVMakeCompressibleOptPass(*PR);
@@ -275,6 +276,7 @@ class RISCVPassConfig : public TargetPassConfig {
   bool addIRTranslator() override;
   void addPreLegalizeMachineIR() override;
   bool addLegalizeMachineIR() override;
+  void addPreRegBankSelect() override;
   bool addRegBankSelect() override;
   bool addGlobalInstructionSelect() override;
   void addPreEmitPass() override;
@@ -345,6 +347,11 @@ bool RISCVPassConfig::addLegalizeMachineIR() {
   return false;
 }
 
+void RISCVPassConfig::addPreRegBankSelect() {
+  if (getOptLevel() != CodeGenOptLevel::None)
+    addPass(createRISCVPostLegalizerCombiner());
+}
+
 bool RISCVPassConfig::addRegBankSelect() {
   addPass(new RegBankSelect());
   return false;
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll b/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll
index 9557f581be4ecec..4fe1691714bb958 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll
@@ -391,7 +391,6 @@ define i64 @add_i64(i64 %a, i64 %b) {
 ; RV32IM-NEXT:    add a0, a0, a2
 ; RV32IM-NEXT:    sltu a2, a0, a2
 ; RV32IM-NEXT:    add a1, a1, a3
-; RV32IM-NEXT:    andi a2, a2, 1
 ; RV32IM-NEXT:    add a1, a1, a2
 ; RV32IM-NEXT:    ret
 ;
@@ -409,8 +408,6 @@ define i64 @addi_i64(i64 %a) {
 ; RV32IM:       # %bb.0: # %entry
 ; RV32IM-NEXT:    addi a0, a0, 1234
 ; RV32IM-NEXT:    sltiu a2, a0, 1234
-; RV32IM-NEXT:    mv a1, a1
-; RV32IM-NEXT:    andi a2, a2, 1
 ; RV32IM-NEXT:    add a1, a1, a2
 ; RV32IM-NEXT:    ret
 ;
@@ -429,7 +426,6 @@ define i64 @sub_i64(i64 %a, i64 %b) {
 ; RV32IM-NEXT:    sub a4, a0, a2
 ; RV32IM-NEXT:    sltu a0, a0, a2
 ; RV32IM-NEXT:    sub a1, a1, a3
-; RV32IM-NEXT:    andi a0, a0, 1
 ; RV32IM-NEXT:    sub a1, a1, a0
 ; RV32IM-NEXT:    mv a0, a4
 ; RV32IM-NEXT:    ret
@@ -450,8 +446,6 @@ define i64 @subi_i64(i64 %a) {
 ; RV32IM-NEXT:    addi a3, a2, 1548
 ; RV32IM-NEXT:    sub a2, a0, a3
 ; RV32IM-NEXT:    sltu a0, a0, a3
-; RV32IM-NEXT:    mv a1, a1
-; RV32IM-NEXT:    andi a0, a0, 1
 ; RV32IM-NEXT:    sub a1, a1, a0
 ; RV32IM-NEXT:    mv a0, a2
 ; RV32IM-NEXT:    ret
@@ -489,7 +483,7 @@ define i64 @andi_i64(i64 %a) {
 ; RV32IM-LABEL: andi_i64:
 ; RV32IM:       # %bb.0: # %entry
 ; RV32IM-NEXT:    andi a0, a0, 1234
-; RV32IM-NEXT:    andi a1, a1, 0
+; RV32IM-NEXT:    li a1, 0
 ; RV32IM-NEXT:    ret
 ;
 ; RV64IM-LABEL: andi_i64:
@@ -521,7 +515,6 @@ define i64 @ori_i64(i64 %a) {
 ; RV32IM-LABEL: ori_i64:
 ; RV32IM:       # %bb.0: # %entry
 ; RV32IM-NEXT:    ori a0, a0, 1234
-; RV32IM-NEXT:    ori a1, a1, 0
 ; RV32IM-NEXT:    ret
 ;
 ; RV64IM-LABEL: ori_i64:
@@ -553,7 +546,6 @@ define i64 @xori_i64(i64 %a) {
 ; RV32IM-LABEL: xori_i64:
 ; RV32IM:       # %bb.0: # %entry
 ; RV32IM-NEXT:    xori a0, a0, 1234
-; RV32IM-NEXT:    xori a1, a1, 0
 ; RV32IM-NEXT:    ret
 ;
 ; RV64IM-LABEL: xori_i64:
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
index 016779689d737f2..82c66c23b26ba94 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/gisel-commandline-option.ll
@@ -21,6 +21,8 @@
 ; ENABLED-NEXT:  Analysis containing CSE Info
 ; ENABLED-O1-NEXT:  RISCVPreLegalizerCombiner
 ; ENABLED-NEXT:  Legalizer
+; ENABLED-O1-NEXT:  MachineDominator Tree Construction
+; ENABLED-O1-NEXT:  RISCVPostLegalizerCombiner
 ; ENABLED-NEXT:  RegBankSelect
 ; ENABLED-NEXT:  Analysis for ComputingKnownBits
 ; ENABLED-O1-NEXT:  Lazy Branch Probability Analysis

@topperc topperc merged commit 8e87dc1 into llvm:main Sep 22, 2023
4 checks passed
@topperc topperc deleted the pr/postlegalizer-combine branch September 22, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants