WIP/RFC: Generic MachineInstr convenience wrappers.

[aemerson]

Summary

Instead of code patterns like:

if (MI.getOpcode() == TargetOpcode::G_LOAD)
  Ptr = MI.getOperand(1);

having some wrappers could make the opcode checking a bit shorter, and also allow us to define some abstractions over the raw MachineInstr specific to a particular generic operation. Here's an example of how that could work with G_LOAD, using automatic bool and MachineInstr* conversions.

[paquette]

llvm/include/llvm/CodeGen/GlobalISel/GenericInstWrapper.h

42

I feel like "load reg" isn't really descriptive.

Maybe getDstReg() would be better? That would also make it easy to share terminology with other wrappers.

[aemerson]

One additional use for this could be as a replacement for passing around MachineInstr when a specific operation is expected.

Register doFooOnLoad(MachineInstr &MI) {
  assert(MI.getOpcode() == TargetOpcode::G_LOAD);
  ...

could be:

Register dooFooOnLoad(GLoad Load) {
  Load.validate();
  ...

llvm/include/llvm/CodeGen/GlobalISel/GenericInstWrapper.h

42

Yeah, this is just a proposal, the specific names we could work out later (if we decide to do ahead at all).

[arsenm]

I've thought we should have something that looks like llvm::Instruction and SDNode hiearachies so you can still do things like dyn_cast to a specific instruction type. Plus the current getOperand+getReg is really ugly given that most every operation can only use registers.

Can you change this to make dyn_cast work, rather than having to construct a wrapper class? I guess that would add a GMI subclass to MachineInstr, and then opcode specific subclasses of that.

[aemerson]

In D105714#2868345, @arsenm wrote:

I've thought we should have something that looks like llvm::Instruction and SDNode hiearachies so you can still do things like dyn_cast to a specific instruction type. Plus the current getOperand+getReg is really ugly given that most every operation can only use registers.

Can you change this to make dyn_cast work, rather than having to construct a wrapper class? I guess that would add a GMI subclass to MachineInstr, and then opcode specific subclasses of that.

In D105714#2868345, @arsenm wrote:

I've thought we should have something that looks like llvm::Instruction and SDNode hiearachies so you can still do things like dyn_cast to a specific instruction type. Plus the current getOperand+getReg is really ugly given that most every operation can only use registers.

Can you change this to make dyn_cast work, rather than having to construct a wrapper class? I guess that would add a GMI subclass to MachineInstr, and then opcode specific subclasses of that.

Sure, I've made a separate patch for that discussion in D105751

llvm/include/llvm/CodeGen/GlobalISel/GenericInstWrapper.h

+//===- llvm/CodeGen/GlobalISel/GenericInstWrapper.h -------------*- 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
+/// Declares convenience wrapper classes for interpreting MachineInstr instances
+/// as specific generic operations.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_GLOBALISEL_GENERICINSTWRAPPER_H
+#define LLVM_CODEGEN_GLOBALISEL_GENERICINSTWRAPPER_H
+
+#include "llvm/CodeGen/MachineInstr.h"
+
+namespace llvm {
+
+/// A base class for all wrappers to use. This provides the opcode checking
+/// logic and common utilities.
+class GInstWrapper {
+protected:
+  MachineInstr *Inst;
+
+public:
+  GInstWrapper(MachineInstr *MI, unsigned Opc) : Inst(MI) {
+    if (MI && MI->getOpcode() != Opc)
+      Inst = nullptr;
+  }
+  operator bool() { return Inst != nullptr; }
+  operator MachineInstr*() { return Inst; }
+  MachineInstr *getMI() { return Inst; }
+};
+
+class GLoad : public GInstWrapper {
+public:
+  GLoad(MachineInstr *MI) : GInstWrapper(MI, TargetOpcode::G_LOAD) {}
+
+  /// Get the definition register of the loaded value.
+  Register getLoadReg() const { return Inst->getOperand(0).getReg(); }
+  /// Get the source register of the pointer value.
+  Register getPointerReg() const { return Inst->getOperand(1).getReg(); }
+  MachineInstr::mmo_iterator memoperands_begin() {
+    return Inst->memoperands_begin();
+  }
+  MachineInstr::mmo_iterator memoperands_end() {
+    return Inst->memoperands_end();
+  }
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_GLOBALISEL_GENERICINSTWRAPPER_H
 No newline at end of file

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

 //===-- lib/CodeGen/GlobalISel/GICombinerHelper.cpp -----------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 #include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/CodeGen/GlobalISel/Combiner.h"
+#include "llvm/CodeGen/GlobalISel/GenericInstWrapper.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/GISelKnownBits.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/MIPatternMatch.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/LowLevelType.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineDominators.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
bool CombinerHelper::matchSextInRegOfLoad(
     MachineInstr &MI, std::tuple<Register, unsigned> &MatchInfo) {
   assert(MI.getOpcode() == TargetOpcode::G_SEXT_INREG);
 
   // Only supports scalars for now.
   if (MRI.getType(MI.getOperand(0).getReg()).isVector())
     return false;
 
   Register SrcReg = MI.getOperand(1).getReg();
-  MachineInstr *LoadDef = getOpcodeDef(TargetOpcode::G_LOAD, SrcReg, MRI);
-  if (!LoadDef || !MRI.hasOneNonDBGUse(LoadDef->getOperand(0).getReg()))
+  auto Load = GLoad(getOpcodeDef(TargetOpcode::G_LOAD, SrcReg, MRI));
+  if (!Load || !MRI.hasOneNonDBGUse(Load.getLoadReg()))
     return false;
 
   // If the sign extend extends from a narrower width than the load's width,
   // then we can narrow the load width when we combine to a G_SEXTLOAD.
-  auto &MMO = **LoadDef->memoperands_begin();
+  auto &MMO = **Load.memoperands_begin();
   // Don't do this for non-simple loads.
   if (MMO.isAtomic() || MMO.isVolatile())
     return false;
 
   // Avoid widening the load at all.
   unsigned NewSizeBits =
       std::min((uint64_t)MI.getOperand(2).getImm(), MMO.getSizeInBits());
 
   // Don't generate G_SEXTLOADs with a < 1 byte width.
   if (NewSizeBits < 8)
     return false;
   // Don't bother creating a non-power-2 sextload, it will likely be broken up
   // anyway for most targets.
   if (!isPowerOf2_32(NewSizeBits))
     return false;
-  MatchInfo = std::make_tuple(LoadDef->getOperand(0).getReg(), NewSizeBits);
+  MatchInfo = std::make_tuple(Load.getLoadReg(), NewSizeBits);
   return true;
 }
 
 void CombinerHelper::applySextInRegOfLoad(
     MachineInstr &MI, std::tuple<Register, unsigned> &MatchInfo) {
   assert(MI.getOpcode() == TargetOpcode::G_SEXT_INREG);
   Register LoadReg;
   unsigned ScalarSizeBits;
   std::tie(LoadReg, ScalarSizeBits) = MatchInfo;
-  auto *LoadDef = MRI.getVRegDef(LoadReg);
-  assert(LoadDef && "Expected a load reg");
+  auto Load = GLoad(MRI.getVRegDef(LoadReg));
+  assert(Load && "Expected a load reg");
 
   // If we have the following:
   // %ld = G_LOAD %ptr, (load 2)
   // %ext = G_SEXT_INREG %ld, 8
   //    ==>
   // %ld = G_SEXTLOAD %ptr (load 1)
 
-  auto &MMO = **LoadDef->memoperands_begin();
-  Builder.setInstrAndDebugLoc(*LoadDef);
+  auto &MMO = **Load.memoperands_begin();
+  Builder.setInstrAndDebugLoc(*Load);
   auto &MF = Builder.getMF();
   auto PtrInfo = MMO.getPointerInfo();
   auto *NewMMO = MF.getMachineMemOperand(&MMO, PtrInfo, ScalarSizeBits / 8);
   Builder.buildLoadInstr(TargetOpcode::G_SEXTLOAD, MI.getOperand(0).getReg(),
-                         LoadDef->getOperand(1).getReg(), *NewMMO);
+                         Load.getPointerReg(), *NewMMO);
   MI.eraseFromParent();
 }
 
 bool CombinerHelper::findPostIndexCandidate(MachineInstr &MI, Register &Addr,
                                             Register &Base, Register &Offset) {
   auto &MF = *MI.getParent()->getParent();
   const auto &TLI = *MF.getSubtarget().getTargetLowering();