GlobalISel: Introduce GenericMachineInstr classes and derivatives for idiomatic LLVM RTTI.

[aemerson]

Summary

This adds some level of type safety, allows helper functions to be added for specific opcodes for free, and also allows us to succinctly check for class membership with the usual dyn_cast/isa/cast functions.

To start off with, add variants for the different load/store operations with some places using it.

[paquette]

I think I like this direction more just because the casting stuff is more consistent with the rest of LLVM. That might be more approachable for people who are more experienced with other parts of LLVM.

[aditya_nandakumar]

In D105751#2868526, @paquette wrote:

I think I like this direction more just because the casting stuff is more consistent with the rest of LLVM. That might be more approachable for people who are more experienced with other parts of LLVM.

+1 from me.

[tschuett]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

24

Does this need to be public?

29

Can this be protected?

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

24

Yes, all the public members of MachineInstr need to be accessible.

29

Likewise, this also needs to be accessible outside.

[aemerson]

Convert some explicit checking of MI.getOpcode() to use the new types.

Also add a templated variant of getOpcodeDef() that takes the class to match instead of an opcode.

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

25

Can we get away with allocating these as regular MachineInstrs since there are no real members?

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

62

Also getPtrReg? A get-single-mmo wrapper would be nice too

113

Other places refer to this as the value reg

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

25

Not sure what you mean. Have the others inherit directly from MachineInstr? If so we wouldn't be able to declare GISel helpers like getReg(), unless we add those to MachineInstr. Even then, it would be useful to have this to enforce more type safety in future for places which only expect to deal with generic instructions.

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

25

I mean is it legal C++ to do:

X = new MachineInstr()
Y = cast<GenericMachineInstr>(X)

with guaranteed sizeof(*X) == sizeof(*Y), or would we need to allocate these initially as GenericMachineInstr?

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

25

Yes that's legal as long as we don't store any data in GenericMachineInstr or its derivatives. I think the same mechanism is already used in the IR, e.g. IntrinsicInst is a convenience class around CallInst that just provides classof and other helpers, like we're doing here.

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

62

Already provided in the base class GLoadStore.

I'll add a getFirstMMO() helper. We can also add more things like isAtomic() and isVolatile().

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

62

Only one is legal, so no first

[aemerson]

Add:

getMMO()
getMemSizeInBits()
getMemSize()
isAtomic()
isVolatile()

and use where possible.

[paquette]

Maybe an isSimple() would be handy too?

(There are some combines in the DAGCombiner which use a similarly-named function.)

[aemerson]

Add isSimple(), and rename GStore::getStoredReg() -> GStore::getValueReg()

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

600 ↗

(On Diff #358105)

Why isn't this GAnyLoad?

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

600 ↗

(On Diff #358105)

To retain existing behaviour, this function only matches zero extending loads. I wanted to have the function signature reflect the restricted opcode we expect to have returned.

[paquette]

I think this looks fine at this point. Added a couple nits but that's about it.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

701 ↗

(On Diff #358105)

This comment seems redundant now.

702 ↗

(On Diff #358105)

Now that we don't need to pull out MMO explicitly, I think this can be folded into the previous if.

llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h

+//===- llvm/CodeGen/GlobalISel/GenericMachineInstrs.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_GENERICMACHINEINSTRS_H
+#define LLVM_CODEGEN_GLOBALISEL_GENERICMACHINEINSTRS_H
+
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/TargetOpcodes.h"
+#include "llvm/Support/Casting.h"
+
+namespace llvm {
+
+/// A base class for all GenericMachineInstrs.
+class GenericMachineInstr : public MachineInstr {
+public:
+  GenericMachineInstr() = delete;
+
+  /// Access the Idx'th operand as a register and return it.
+  Register getReg(unsigned Idx) { return getOperand(Idx).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    return isPreISelGenericOpcode(MI->getOpcode());
+  }
+};
+
+class GLoadStore : public GenericMachineInstr {
+public:
+  /// Get the source register of the pointer value.
+  Register getPointerReg() const { return getOperand(1).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    switch (MI->getOpcode()) {
+    case TargetOpcode::G_LOAD:
+    case TargetOpcode::G_STORE:
+    case TargetOpcode::G_ZEXTLOAD:
+    case TargetOpcode::G_SEXTLOAD:
+      return true;
+    default:
+      return false;
+    }
+  }
+};
+
+class GAnyLoad : public GLoadStore {
+public:
+  /// Get the definition register of the loaded value.
+  Register getDstReg() const { return getOperand(0).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    switch (MI->getOpcode()) {
+    case TargetOpcode::G_LOAD:
+    case TargetOpcode::G_ZEXTLOAD:
+    case TargetOpcode::G_SEXTLOAD:
+      return true;
+    default:
+      return false;
+    }
+  }
+};
+
+class GLoad : public GAnyLoad {
+public:
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_LOAD;
+  }
+};
+class GSExtLoad : public GAnyLoad {
+public:
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_SEXTLOAD;
+  }
+};
+class GZExtLoad : public GAnyLoad {
+public:
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_ZEXTLOAD;
+  }
+};
+
+class GStore : public GLoadStore {
+public:
+  /// Get the stored value register.
+  Register getStoredReg() const { return getOperand(0).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_STORE;
+  }
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_GLOBALISEL_GENERICMACHINEINSTRS_H
 No newline at end of file

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

 //===-- llvm/CodeGen/GlobalISel/LegalizerHelper.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
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file This file implements the LegalizerHelper class to legalize
 /// individual instructions and the LegalizeMachineIR wrapper pass for the
 /// primary legalization.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
+#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/LostDebugLocObserver.h"
 #include "llvm/CodeGen/GlobalISel/MIPatternMatch.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
if (NewNumElts < OldNumElts) {
     MI.eraseFromParent();
     return Legalized;
   }
 
   return UnableToLegalize;
 }
 
 LegalizerHelper::LegalizeResult
-LegalizerHelper::lowerLoad(MachineInstr &MI) {
+LegalizerHelper::lowerLoad(MachineInstr &LoadMI) {
   // Lower to a memory-width G_LOAD and a G_SEXT/G_ZEXT/G_ANYEXT
-  Register DstReg = MI.getOperand(0).getReg();
-  Register PtrReg = MI.getOperand(1).getReg();
+  auto *MI = cast<GAnyLoad>(&LoadMI);
+  Register DstReg = MI->getDstReg();
+  Register PtrReg = MI->getPointerReg();
   LLT DstTy = MRI.getType(DstReg);
-  MachineMemOperand &MMO = **MI.memoperands_begin();
+  MachineMemOperand &MMO = **MI->memoperands_begin();
   LLT MemTy = MMO.getMemoryType();
   MachineFunction &MF = MIRBuilder.getMF();
   if (MemTy.isVector())
     return UnableToLegalize;
 
   unsigned MemSizeInBits = MemTy.getSizeInBits();
   unsigned MemStoreSizeInBits = 8 * MemTy.getSizeInBytes();
 
if (MemSizeInBits != MemStoreSizeInBits) {
 
     // If this wasn't already an extending load, we need to widen the result
     // register to avoid creating a load with a narrower result than the source.
     if (MemStoreSizeInBits > DstTy.getSizeInBits()) {
       LoadTy = WideMemTy;
       LoadReg = MRI.createGenericVirtualRegister(WideMemTy);
     }
 
-    if (MI.getOpcode() == TargetOpcode::G_SEXTLOAD) {
+    if (isa<GSExtLoad>(MI)) {
       auto NewLoad = MIRBuilder.buildLoad(LoadTy, PtrReg, *NewMMO);
       MIRBuilder.buildSExtInReg(LoadReg, NewLoad, MemSizeInBits);
-    } else if (MI.getOpcode() == TargetOpcode::G_ZEXTLOAD ||
-               WideMemTy == DstTy) {
+    } else if (isa<GZExtLoad>(MI) || WideMemTy == DstTy) {
       auto NewLoad = MIRBuilder.buildLoad(LoadTy, PtrReg, *NewMMO);
       // The extra bits are guaranteed to be zero, since we stored them that
       // way.  A zext load from Wide thus automatically gives zext from MemVT.
       MIRBuilder.buildAssertZExt(LoadReg, NewLoad, MemSizeInBits);
     } else {
       MIRBuilder.buildLoad(LoadReg, PtrReg, *NewMMO);
     }
 
     if (DstTy != LoadTy)
       MIRBuilder.buildTrunc(DstReg, LoadReg);
 
-    MI.eraseFromParent();
+    MI->eraseFromParent();
     return Legalized;
   }
 
   if (DstTy.getSizeInBits() != MMO.getSizeInBits())
     return UnableToLegalize;
 
-  if (MI.getOpcode() == TargetOpcode::G_LOAD) {
+  if (isa<GLoad>(MI)) {
     // This load needs splitting into power of 2 sized loads.
     if (DstTy.isVector())
       return UnableToLegalize;
     if (isPowerOf2_32(DstTy.getSizeInBits()))
       return UnableToLegalize; // Don't know what we're being asked to do.
 
     // Our strategy here is to generate anyextending loads for the smaller
     // types up to next power-2 result type, and then combine the two larger
auto SmallPtr =
       MIRBuilder.buildPtrAdd(PtrAddReg, PtrReg, OffsetCst);
     auto SmallLoad = MIRBuilder.buildLoad(AnyExtTy, SmallPtr,
                                           *SmallMMO);
 
     auto ShiftAmt = MIRBuilder.buildConstant(AnyExtTy, LargeSplitSize);
     auto Shift = MIRBuilder.buildShl(AnyExtTy, SmallLoad, ShiftAmt);
     auto Or = MIRBuilder.buildOr(AnyExtTy, Shift, LargeLoad);
     MIRBuilder.buildTrunc(DstReg, {Or});
-    MI.eraseFromParent();
+    MI->eraseFromParent();
     return Legalized;
   }
 
   return UnableToLegalize;
 }
 
 LegalizerHelper::LegalizeResult
 LegalizerHelper::lowerStore(MachineInstr &MI) {

 }
 
 LegalizerHelper::LegalizeResult
 LegalizerHelper::lowerAbsToAddXor(MachineInstr &MI) {
   // Expand %res = G_ABS %a into:
   // %v1 = G_ASHR %a, scalar_size-1
   // %v2 = G_ADD %a, %v1
   // %res = G_XOR %v2, %v1
-  LLT DstTy = MRI.getType(MI.getOperand(0).getReg());
+  auto *GMI = cast<GenericMachineInstr>(&MI);
+  LLT DstTy = MRI.getType(GMI->getReg(0));
   Register OpReg = MI.getOperand(1).getReg();
   auto ShiftAmt =
       MIRBuilder.buildConstant(DstTy, DstTy.getScalarSizeInBits() - 1);
   auto Shift = MIRBuilder.buildAShr(DstTy, OpReg, ShiftAmt);
   auto Add = MIRBuilder.buildAdd(DstTy, OpReg, Shift);
-  MIRBuilder.buildXor(MI.getOperand(0).getReg(), Add, Shift);
+  MIRBuilder.buildXor(GMI->getReg(0), Add, Shift);
   MI.eraseFromParent();
   return Legalized;
 }
 
 LegalizerHelper::LegalizeResult
 LegalizerHelper::lowerAbsToMaxNeg(MachineInstr &MI) {
   // Expand %res = G_ABS %a into:
   // %v1 = G_CONSTANT 0