[RISCV] Merge SExtWRemoval and StripWSuffix into a single pass.

[craig.topper]

Summary

These run together in the pipeline and are the only users of
TII.hasAllWUsers. Merging them will allow us to move hasAllWUsers
back from TII.

[asb]

LGTM. I'd note that clang-format prefers different formatting for some of the functions here - it's probably worth reformatting while you're doing this merge into a new pass.

llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp

9–18

couple => couple of

[jrtc27]

In D147173#4231929, @asb wrote:

LGTM. I'd note that clang-format prefers different formatting for some of the functions here - it's probably worth reformatting while you're doing this merge into a new pass.

As a separate commit though, please, so the diffs are clearer

[craig.topper]

In D147173#4231929, @asb wrote:

LGTM. I'd note that clang-format prefers different formatting for some of the functions here - it's probably worth reformatting while you're doing this merge into a new pass.

Is it just the switch statement I intentionally wrote with the lines compacted?

[asb]

In D147173#4232034, @craig.topper wrote:

In D147173#4231929, @asb wrote:

LGTM. I'd note that clang-format prefers different formatting for some of the functions here - it's probably worth reformatting while you're doing this merge into a new pass.

Is it just the switch statement I intentionally wrote with the lines compacted?

There were other instances too, but I ran it after applying D147174 so may have been from changed lines there. As for the switch - it's short enough I personally wouldn't bother opting out of clang-format's preferences, but I don't have a problem with it as is.

[craig.topper]

In D147173#4232115, @asb wrote:

In D147173#4232034, @craig.topper wrote:

In D147173#4231929, @asb wrote:

LGTM. I'd note that clang-format prefers different formatting for some of the functions here - it's probably worth reformatting while you're doing this merge into a new pass.

Is it just the switch statement I intentionally wrote with the lines compacted?

There were other instances too, but I ran it after applying D147174 so may have been from changed lines there. As for the switch - it's short enough I personally wouldn't bother opting out of clang-format's preferences, but I don't have a problem with it as is.

Ok I fixed D147174 when I committed it.

For the switch I like it on one line because it makes it easy to see the correspondence between W and non-W across the line. And having the components lined up for each line.

llvm/lib/Target/RISCV/CMakeLists.txt

add_llvm_target(RISCVCodeGen
   RISCVInsertVSETVLI.cpp
   RISCVInstrInfo.cpp
   RISCVISelDAGToDAG.cpp
   RISCVISelLowering.cpp
   RISCVMachineFunctionInfo.cpp
   RISCVMacroFusion.cpp
   RISCVMCInstLower.cpp
   RISCVMergeBaseOffset.cpp
+  RISCVOptWInstrs.cpp
   RISCVRedundantCopyElimination.cpp
   RISCVRegisterInfo.cpp
   RISCVRVVInitUndef.cpp
-  RISCVSExtWRemoval.cpp
-  RISCVStripWSuffix.cpp
   RISCVSubtarget.cpp
   RISCVTargetMachine.cpp
   RISCVTargetObjectFile.cpp
   RISCVTargetTransformInfo.cpp
   GISel/RISCVCallLowering.cpp
   GISel/RISCVInstructionSelector.cpp
   GISel/RISCVLegalizerInfo.cpp
   GISel/RISCVRegisterBankInfo.cpp

llvm/lib/Target/RISCV/RISCV.h

FunctionPass *createRISCVISelDag(RISCVTargetMachine &TM,
                                  CodeGenOpt::Level OptLevel);
 
 FunctionPass *createRISCVMakeCompressibleOptPass();
 void initializeRISCVMakeCompressibleOptPass(PassRegistry &);
 
 FunctionPass *createRISCVGatherScatterLoweringPass();
 void initializeRISCVGatherScatterLoweringPass(PassRegistry &);
 
-FunctionPass *createRISCVSExtWRemovalPass();
-void initializeRISCVSExtWRemovalPass(PassRegistry &);
-
-FunctionPass *createRISCVStripWSuffixPass();
-void initializeRISCVStripWSuffixPass(PassRegistry &);
+FunctionPass *createRISCVOptWInstrsPass();
+void initializeRISCVOptWInstrsPass(PassRegistry &);
 
 FunctionPass *createRISCVMergeBaseOffsetOptPass();
 void initializeRISCVMergeBaseOffsetOptPass(PassRegistry &);
 
 FunctionPass *createRISCVExpandPseudoPass();
 void initializeRISCVExpandPseudoPass(PassRegistry &);
 
 FunctionPass *createRISCVPreRAExpandPseudoPass();

llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp

-//===-------------- RISCVSExtWRemoval.cpp - MI sext.w Removal -------------===//
+//===- RISCVOptWInstrs.cpp - MI W instruction optimizations ---------------===//
 //
 // 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
 //
 //===---------------------------------------------------------------------===//
 //
-// This pass removes unneeded sext.w instructions at the MI level. Either
-// because the sign extended bits aren't consumed or because the input was
-// already sign extended by an earlier instruction.
+// This pass does some optimizations for *W instructions at the MI level.
+//
+// First it removes unneeded sext.w instructions. Either because the sign
+// extended bits aren't consumed or because the input was already sign extended
+// by an earlier instruction.
+//
+// Then it removes the -w suffix from each addiw and slliw instructions
+// whenever all users are dependent only on the lower word of the result of the
+// instruction. We do this only for addiw, slliw, and mulw because the -w forms
+// are less compressible.
 //
 //===---------------------------------------------------------------------===//
 
 #include "RISCV.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 
 using namespace llvm;
 
-#define DEBUG_TYPE "riscv-sextw-removal"
+#define DEBUG_TYPE "riscv-opt-w-instrs"
+#define RISCV_OPT_W_INSTRS_NAME "RISC-V Optimize W Instructions"
 
 STATISTIC(NumRemovedSExtW, "Number of removed sign-extensions");
 STATISTIC(NumTransformedToWInstrs,
           "Number of instructions transformed to W-ops");
 
 static cl::opt<bool> DisableSExtWRemoval("riscv-disable-sextw-removal",
                                          cl::desc("Disable removal of sext.w"),
                                          cl::init(false), cl::Hidden);
+static cl::opt<bool> DisableStripWSuffix("riscv-disable-strip-w-suffix",
+                                         cl::desc("Disable strip W suffix"),
+                                         cl::init(false), cl::Hidden);
+
 namespace {
 
-class RISCVSExtWRemoval : public MachineFunctionPass {
+class RISCVOptWInstrs : public MachineFunctionPass {
 public:
   static char ID;
 
-  RISCVSExtWRemoval() : MachineFunctionPass(ID) {
-    initializeRISCVSExtWRemovalPass(*PassRegistry::getPassRegistry());
+  RISCVOptWInstrs() : MachineFunctionPass(ID) {
+    initializeRISCVOptWInstrsPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnMachineFunction(MachineFunction &MF) override;
+  bool removeSExtWInstrs(MachineFunction &MF, const RISCVInstrInfo &TII,
+                         MachineRegisterInfo &MRI);
+  bool stripWSuffixes(MachineFunction &MF, const RISCVInstrInfo &TII,
+                      MachineRegisterInfo &MRI);
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
     MachineFunctionPass::getAnalysisUsage(AU);
   }
 
-  StringRef getPassName() const override { return "RISC-V sext.w Removal"; }
+  StringRef getPassName() const override { return RISCV_OPT_W_INSTRS_NAME; }
 };
 
 } // end anonymous namespace
 
-char RISCVSExtWRemoval::ID = 0;
-INITIALIZE_PASS(RISCVSExtWRemoval, DEBUG_TYPE, "RISC-V sext.w Removal", false,
+char RISCVOptWInstrs::ID = 0;
+INITIALIZE_PASS(RISCVOptWInstrs, DEBUG_TYPE, RISCV_OPT_W_INSTRS_NAME, false,
                 false)
 
-FunctionPass *llvm::createRISCVSExtWRemovalPass() {
-  return new RISCVSExtWRemoval();
+FunctionPass *llvm::createRISCVOptWInstrsPass() {
+  return new RISCVOptWInstrs();
 }
 
 // This function returns true if the machine instruction always outputs a value
 // where bits 63:32 match bit 31.
 static bool isSignExtendingOpW(const MachineInstr &MI,
                                const MachineRegisterInfo &MRI) {
   uint64_t TSFlags = MI.getDesc().TSFlags;
 
case RISCV::SLLI:
     return RISCV::SLLIW;
   case RISCV::SUB:
     return RISCV::SUBW;
   default:
     llvm_unreachable("Unexpected opcode for replacement with W variant");
   }
 }
 
-bool RISCVSExtWRemoval::runOnMachineFunction(MachineFunction &MF) {
-  if (skipFunction(MF.getFunction()) || DisableSExtWRemoval)
-    return false;
-
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
-  const RISCVInstrInfo &TII = *ST.getInstrInfo();
-
-  if (!ST.is64Bit())
+bool RISCVOptWInstrs::removeSExtWInstrs(MachineFunction &MF,
+                                        const RISCVInstrInfo &TII,
+                                        MachineRegisterInfo &MRI) {
+  if (DisableSExtWRemoval)
     return false;
 
   bool MadeChange = false;
-
   for (MachineBasicBlock &MBB : MF) {
     for (auto I = MBB.begin(), IE = MBB.end(); I != IE;) {
       MachineInstr *MI = &*I++;
 
       // We're looking for the sext.w pattern ADDIW rd, rs1, 0.
       if (!RISCV::isSEXT_W(*MI))
         continue;
 
for (auto I = MBB.begin(), IE = MBB.end(); I != IE;) {
       MI->eraseFromParent();
       ++NumRemovedSExtW;
       MadeChange = true;
     }
   }
 
   return MadeChange;
 }
+
+bool RISCVOptWInstrs::stripWSuffixes(MachineFunction &MF,
+                                     const RISCVInstrInfo &TII,
+                                     MachineRegisterInfo &MRI) {
+  if (DisableStripWSuffix)
+    return false;
+
+  bool MadeChange = false;
+  for (MachineBasicBlock &MBB : MF) {
+    for (auto I = MBB.begin(), IE = MBB.end(); I != IE; ++I) {
+      MachineInstr &MI = *I;
+
+      unsigned Opc;
+      switch (MI.getOpcode()) {
+      default:
+        continue;
+      case RISCV::ADDW:  Opc = RISCV::ADD;  break;
+      case RISCV::MULW:  Opc = RISCV::MUL;  break;
+      case RISCV::SLLIW: Opc = RISCV::SLLI; break;
+      }
+
+      if (TII.hasAllWUsers(MI, MRI)) {
+        MI.setDesc(TII.get(Opc));
+        MadeChange = true;
+      }
+    }
+  }
+
+  return MadeChange;
+}
+
+bool RISCVOptWInstrs::runOnMachineFunction(MachineFunction &MF) {
+  if (skipFunction(MF.getFunction()))
+    return false;
+
+  MachineRegisterInfo &MRI = MF.getRegInfo();
+  const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
+  const RISCVInstrInfo &TII = *ST.getInstrInfo();
+
+  if (!ST.is64Bit())
+    return false;
+
+  bool MadeChange = false;
+  MadeChange |= removeSExtWInstrs(MF, TII, MRI);
+  MadeChange |= stripWSuffixes(MF, TII, MRI);
+
+  return MadeChange;
+}

llvm/lib/Target/RISCV/RISCVStripWSuffix.cpp

-//===-------------- RISCVStripWSuffix.cpp - -w Suffix Removal -------------===//
-//
-//
-// 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
-//
-//===---------------------------------------------------------------------===//
-//
-// This pass removes the -w suffix from each addiw and slliw instructions
-// whenever all users are dependent only on the lower word of the result of the
-// instruction. We do this only for addiw and slliw because the -w forms are
-// less compressible.
-//
-//===---------------------------------------------------------------------===//
-
-#include "RISCV.h"
-#include "RISCVMachineFunctionInfo.h"
-
-using namespace llvm;
-
-static cl::opt<bool> DisableStripWSuffix("riscv-disable-strip-w-suffix",
-                                         cl::desc("Disable strip W suffix"),
-                                         cl::init(false), cl::Hidden);
-
-namespace {
-
-class RISCVStripWSuffix : public MachineFunctionPass {
-public:
-  static char ID;
-
-  RISCVStripWSuffix() : MachineFunctionPass(ID) {
-    initializeRISCVStripWSuffixPass(*PassRegistry::getPassRegistry());
-  }
-
-  bool runOnMachineFunction(MachineFunction &MF) override;
-
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
-    MachineFunctionPass::getAnalysisUsage(AU);
-  }
-
-  StringRef getPassName() const override { return "RISC-V Strip W Suffix"; }
-};
-
-} // end anonymous namespace
-
-char RISCVStripWSuffix::ID = 0;
-INITIALIZE_PASS(RISCVStripWSuffix, "riscv-strip-w-suffix",
-                "RISC-V Strip W Suffix", false, false)
-
-FunctionPass *llvm::createRISCVStripWSuffixPass() {
-  return new RISCVStripWSuffix();
-}
-
-bool RISCVStripWSuffix::runOnMachineFunction(MachineFunction &MF) {
-  if (skipFunction(MF.getFunction()) || DisableStripWSuffix)
-    return false;
-
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
-  const RISCVInstrInfo &TII = *ST.getInstrInfo();
-
-  if (!ST.is64Bit())
-    return false;
-
-  bool MadeChange = false;
-  for (MachineBasicBlock &MBB : MF) {
-    for (auto I = MBB.begin(), IE = MBB.end(); I != IE; ++I) {
-      MachineInstr &MI = *I;
-
-      unsigned Opc;
-      switch (MI.getOpcode()) {
-      default:
-        continue;
-      case RISCV::ADDW:  Opc = RISCV::ADD;  break;
-      case RISCV::MULW:  Opc = RISCV::MUL;  break;
-      case RISCV::SLLIW: Opc = RISCV::SLLI; break;
-      }
-
-      if (TII.hasAllWUsers(MI, MRI)) {
-        MI.setDesc(TII.get(Opc));
-        MadeChange = true;
-      }
-    }
-  }
-
-  return MadeChange;
-}

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
   RegisterTargetMachine<RISCVTargetMachine> X(getTheRISCV32Target());
   RegisterTargetMachine<RISCVTargetMachine> Y(getTheRISCV64Target());
   auto *PR = PassRegistry::getPassRegistry();
   initializeGlobalISel(*PR);
   initializeRISCVMakeCompressibleOptPass(*PR);
   initializeRISCVGatherScatterLoweringPass(*PR);
   initializeRISCVCodeGenPreparePass(*PR);
   initializeRISCVMergeBaseOffsetOptPass(*PR);
-  initializeRISCVSExtWRemovalPass(*PR);
-  initializeRISCVStripWSuffixPass(*PR);
+  initializeRISCVOptWInstrsPass(*PR);
   initializeRISCVPreRAExpandPseudoPass(*PR);
   initializeRISCVExpandPseudoPass(*PR);
   initializeRISCVInsertVSETVLIPass(*PR);
   initializeRISCVDAGToDAGISelPass(*PR);
   initializeRISCVInitUndefPass(*PR);
 }
 
 static StringRef computeDataLayout(const Triple &TT) {

 }
 
 void RISCVPassConfig::addMachineSSAOptimization() {
   TargetPassConfig::addMachineSSAOptimization();
   if (EnableMachineCombiner)
     addPass(&MachineCombinerID);
 
   if (TM->getTargetTriple().getArch() == Triple::riscv64) {
-    addPass(createRISCVSExtWRemovalPass());
-    addPass(createRISCVStripWSuffixPass());
+    addPass(createRISCVOptWInstrsPass());
   }
 }
 
 void RISCVPassConfig::addPreRegAlloc() {
   addPass(createRISCVPreRAExpandPseudoPass());
   if (TM->getOptLevel() != CodeGenOpt::None)
     addPass(createRISCVMergeBaseOffsetOptPass());
   addPass(createRISCVInsertVSETVLIPass());

llvm/test/CodeGen/RISCV/O3-pipeline.ll

 ; CHECK-NEXT:       MachinePostDominator Tree Construction
 ; CHECK-NEXT:       Machine Cycle Info Analysis
 ; CHECK-NEXT:       Machine code sinking
 ; CHECK-NEXT:       Peephole Optimizations
 ; CHECK-NEXT:       Remove dead machine instructions
 ; CHECK-NEXT:       Machine Trace Metrics
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine InstCombiner
-; RV64-NEXT:        RISC-V sext.w Removal
-; RV64-NEXT:        RISC-V Strip W Suffix
+; RV64-NEXT:        RISC-V Optimize W Instructions
 ; CHECK-NEXT:       RISC-V Pre-RA pseudo instruction expansion pass
 ; CHECK-NEXT:       RISC-V Merge Base Offset
 ; CHECK-NEXT:       RISC-V Insert VSETVLI pass
 ; CHECK-NEXT:       Detect Dead Lanes
 ; CHECK-NEXT:       RISC-V init undef pass
 ; CHECK-NEXT:       Process Implicit Definitions
 ; CHECK-NEXT:       Remove unreachable machine basic blocks
 ; CHECK-NEXT:       Live Variable Analysis