AMDGPU/GlobalISel: Enable TableGen'd instruction selector

[arsenm]

lib/Target/AMDGPU/SIRegisterInfo.cpp

1574 ↗

(On Diff #143661)

Should this be the allocatable SReg_32_XM0?

1577 ↗

(On Diff #143661)

Same for XEXEC

test/CodeGen/AMDGPU/GlobalISel/inst-select-or.mir

4 ↗

(On Diff #143661)

should be able to drop this if the calling convention doesn't matter

[arsenm]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

440 ↗

(On Diff #143661)

Should the default be selectImpl?

[tstellar]

Address code review comments.

[tstellar]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

440 ↗

(On Diff #143661)

Eventually yes, but enabling it for everything now could lead to crashes and miscompiles that will prevent the SelectionDAG fallback from activating. I would like to get a full piglit run working with GISel for simple shaders and SelectionDAG for everything else before we make selectImpl() the default.

[tstellar]

Update TableGen patterns so that they will automatically fold immediates into
the instruction.

[arsenm]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

478 ↗

(On Diff #145852)

We don't fold immediates in the DAG selector now, so why do this here?

[nhaehnle]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

478 ↗

(On Diff #145852)

In fact, I remember there are some edge cases where folding immediates directly doesn't work due to moveToVALU. Basically, if you initially generate an instruction like S_ADD with an immediate and that instruction then gets moved to VALU it becomes a VOP3-encoded V_ADD which cannot have an immediate (because of the restriction that instructions can be at most 64 bits long), leading to machine instruction verifier errors.

The question is whether the same issue may apply here when going through GlobalISel in corner cases, even if we try to be better at selection the correct ALU directly. I think it makes sense to be a bit conservative initially.

[tstellar]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

478 ↗

(On Diff #145852)

We don't fold immediates in the DAG selector now, so why do this here?

Because we can? Ideally doing folding in during ISel would allow us to eliminate or greatly simplify the SIFoldOperands pass, but admittedly, global-isel is so new I don't know if that would be possible. I can go either way on folding during ISel, but it would at least be nice to have a target feature to enable this.

How do you feel about selecting directly to _e32 variants when we can instead of _e64, which also something that global-isel is doing?

[tstellar]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

478 ↗

(On Diff #145852)

Because we can? Ideally doing folding in during ISel would allow us to eliminate or greatly simplify the SIFoldOperands pass, but admittedly, global-isel is so new I don't know if that would be possible. I can go either way on folding during ISel, but it would at least be nice to have a target feature to enable this.

Actually, thinking about this more I would prefer to have global-isel do immediate folding by default with a target option to disable it. global-isel is still very experimental for AMDGPU, and I would like to take advantage of as many new features as possible while it's still in the experimental state. This will also help us learn what's possible and what isn't.

478 ↗

(On Diff #145852)

In theory global-isel won't just be better at selecting the correct ALU, it will *always* select the correct ALU, so we shouldn't have to worry about corner cases like this, but this is all in theory until we actually have a more complete global-isel implementation.

[nhaehnle]

Hmm, you may have a point about trying to push the boundaries of what's possible by folding immediately. It would certainly be nice if it was possible.

[arsenm]

lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

478 ↗

(On Diff #145852)

I don't particularly like either. I don't think doing these during selection eliminates the need to do these things later. Other passes will be introducing and changing instructions, which will introduce new places these need to happen. It's true most of the places that need this now are junk created by SIFixSGPRCopies hacks.

I also think for good debug info constants are supposed to be materialized into a register before use. There are also multiple use and code size considerations when an immediate is used multiple times, and it's probably not appropriate for a single instruction pattern to be considering those, and a proper optimization pass would have to do it.

There is some stuff that has become problematic we do now in SIFoldOperands (the way clamp folding is implemented for example starts to break down with packed operations) that would be better to move when we have more semantically pure pseudoinstructions. Similarly, SDWA would probably be easier do handle during isel (although it has some of the other code size consideration issues as immediates)

Doing folding (especially for encoding shrinking) must be done later, since things like frame index elimination introduce new constants to deal with very late.

[tstellar]

I've dropped the immediate folding for now. This is blocking the rest of
my global isel patches, so I would rather just get this simple version
committed. We can always bring back the immediate folding if we think
it will be useful.

[arsenm]

LGTM. At this point I would be more interested in what source and dest modifier folding looks like in global isel

llvm/trunk/lib/Target/AMDGPU/AMDGPUGISel.td

+//===-- AMDGPUGIsel.td - AMDGPU GlobalISel Patterns---------*- tablegen -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// This files contains patterns that should only be used by GlobalISel.  For
+// example patterns for V_* instructions that have S_* equivalents.
+// SelectionDAG does not support selecting V_* instructions.
+//===----------------------------------------------------------------------===//
+
+include "AMDGPU.td"
+
+def sd_vsrc0 : ComplexPattern<i32, 1, "">;
+def gi_vsrc0 :
+    GIComplexOperandMatcher<s32, "selectVSRC0">,
+    GIComplexPatternEquiv<sd_vsrc0>;
+
+class GISelSop2Pat <
+  SDPatternOperator node,
+  Instruction inst,
+  ValueType dst_vt,
+  ValueType src0_vt = dst_vt, ValueType src1_vt = src0_vt>   : GCNPat <
+
+  (dst_vt (node (src0_vt SReg_32:$src0), (src1_vt SReg_32:$src1))),
+  (inst src0_vt:$src0, src1_vt:$src1)
+>;
+
+class GISelVop2Pat <
+  SDPatternOperator node,
+  Instruction inst,
+  ValueType dst_vt,
+  ValueType src0_vt = dst_vt, ValueType src1_vt = src0_vt>   : GCNPat <
+
+  (dst_vt (node (src0_vt (sd_vsrc0 src0_vt:$src0)), (src1_vt VGPR_32:$src1))),
+  (inst src0_vt:$src0, src1_vt:$src1)
+>;
+
+def : GISelSop2Pat <or, S_OR_B32, i32>;
+def : GISelVop2Pat <or, V_OR_B32_e32, i32>;

llvm/trunk/lib/Target/AMDGPU/AMDGPUInstructionSelector.h

 #ifndef LLVM_LIB_TARGET_AMDGPU_AMDGPUINSTRUCTIONSELECTOR_H
 #define LLVM_LIB_TARGET_AMDGPU_AMDGPUINSTRUCTIONSELECTOR_H
 
 #include "AMDGPU.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 
+namespace {
+#define GET_GLOBALISEL_PREDICATE_BITSET
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_PREDICATE_BITSET
+}
+
 namespace llvm {
 
 class AMDGPUInstrInfo;
 class AMDGPURegisterBankInfo;
+class AMDGPUSubtarget;
 class MachineInstr;
 class MachineOperand;
 class MachineRegisterInfo;
 class SIInstrInfo;
 class SIRegisterInfo;
 class SISubtarget;
 
 class AMDGPUInstructionSelector : public InstructionSelector {
 public:
   AMDGPUInstructionSelector(const SISubtarget &STI,
-                            const AMDGPURegisterBankInfo &RBI);
+                            const AMDGPURegisterBankInfo &RBI,
+                            const AMDGPUTargetMachine &TM);
 
   bool select(MachineInstr &I, CodeGenCoverage &CoverageInfo) const override;
+  static const char *getName();
 
 private:
   struct GEPInfo {
     const MachineInstr &GEP;
     SmallVector<unsigned, 2> SgprParts;
     SmallVector<unsigned, 2> VgprParts;
     int64_t Imm;
     GEPInfo(const MachineInstr &GEP) : GEP(GEP), Imm(0) { }
   };
 
+  /// tblgen-erated 'select' implementation.
+  bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;
+
   MachineOperand getSubOperand64(MachineOperand &MO, unsigned SubIdx) const;
   bool selectG_CONSTANT(MachineInstr &I) const;
   bool selectG_ADD(MachineInstr &I) const;
   bool selectG_GEP(MachineInstr &I) const;
   bool hasVgprParts(ArrayRef<GEPInfo> AddrInfo) const;
   void getAddrModeInfo(const MachineInstr &Load, const MachineRegisterInfo &MRI,
                        SmallVectorImpl<GEPInfo> &AddrInfo) const;
   bool selectSMRD(MachineInstr &I, ArrayRef<GEPInfo> AddrInfo) const;
   bool selectG_LOAD(MachineInstr &I) const;
   bool selectG_STORE(MachineInstr &I) const;
 
+  InstructionSelector::ComplexRendererFns
+  selectVSRC0(MachineOperand &Root) const;
+
   const SIInstrInfo &TII;
   const SIRegisterInfo &TRI;
   const AMDGPURegisterBankInfo &RBI;
+  const AMDGPUTargetMachine &TM;
+  const SISubtarget &STI;
+  bool EnableLateStructurizeCFG;
+#define GET_GLOBALISEL_PREDICATES_DECL
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_PREDICATES_DECL
+
+#define GET_GLOBALISEL_TEMPORARIES_DECL
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_TEMPORARIES_DECL
+
 protected:
   AMDGPUAS AMDGPUASI;
 };
 
 } // End llvm namespace.
 #endif

llvm/trunk/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

 /// \todo This should be generated by TableGen.
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUInstructionSelector.h"
 #include "AMDGPUInstrInfo.h"
 #include "AMDGPURegisterBankInfo.h"
 #include "AMDGPURegisterInfo.h"
 #include "AMDGPUSubtarget.h"
+#include "AMDGPUTargetMachine.h"
+#include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
+#include "llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 #define DEBUG_TYPE "amdgpu-isel"
 
 using namespace llvm;
 
+#define GET_GLOBALISEL_IMPL
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_IMPL
+
 AMDGPUInstructionSelector::AMDGPUInstructionSelector(
-    const SISubtarget &STI, const AMDGPURegisterBankInfo &RBI)
+    const SISubtarget &STI, const AMDGPURegisterBankInfo &RBI,
+    const AMDGPUTargetMachine &TM)
     : InstructionSelector(), TII(*STI.getInstrInfo()),
-      TRI(*STI.getRegisterInfo()), RBI(RBI), AMDGPUASI(STI.getAMDGPUAS()) {}
+      TRI(*STI.getRegisterInfo()), RBI(RBI), TM(TM),
+      STI(STI),
+      EnableLateStructurizeCFG(AMDGPUTargetMachine::EnableLateStructurizeCFG),
+#define GET_GLOBALISEL_PREDICATES_INIT
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_PREDICATES_INIT
+#define GET_GLOBALISEL_TEMPORARIES_INIT
+#include "AMDGPUGenGlobalISel.inc"
+#undef GET_GLOBALISEL_TEMPORARIES_INIT
+      ,AMDGPUASI(STI.getAMDGPUAS())
+{
+}
+
+const char *AMDGPUInstructionSelector::getName() { return DEBUG_TYPE; }
 
 MachineOperand
 AMDGPUInstructionSelector::getSubOperand64(MachineOperand &MO,
                                            unsigned SubIdx) const {
 
   MachineInstr *MI = MO.getParent();
   MachineBasicBlock *BB = MO.getParent()->getParent();
   MachineFunction *MF = BB->getParent();
bool AMDGPUInstructionSelector::select(MachineInstr &I,
                                        CodeGenCoverage &CoverageInfo) const {
 
   if (!isPreISelGenericOpcode(I.getOpcode()))
     return true;
 
   switch (I.getOpcode()) {
   default:
     break;
+  case TargetOpcode::G_OR:
+    return selectImpl(I, CoverageInfo);
   case TargetOpcode::G_ADD:
     return selectG_ADD(I);
   case TargetOpcode::G_CONSTANT:
     return selectG_CONSTANT(I);
   case TargetOpcode::G_GEP:
     return selectG_GEP(I);
   case TargetOpcode::G_LOAD:
     return selectG_LOAD(I);
   case TargetOpcode::G_STORE:
     return selectG_STORE(I);
   }
   return false;
 }
+
+///
+/// This will select either an SGPR or VGPR operand and will save us from
+/// having to write an extra tablegen pattern.
+InstructionSelector::ComplexRendererFns
+AMDGPUInstructionSelector::selectVSRC0(MachineOperand &Root) const {
+  return {{
+      [=](MachineInstrBuilder &MIB) { MIB.add(Root); }
+  }};
+}

llvm/trunk/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

SISubtarget::SISubtarget(const Triple &TT, StringRef GPU, StringRef FS,
     : AMDGPUSubtarget(TT, GPU, FS, TM), InstrInfo(*this),
       FrameLowering(TargetFrameLowering::StackGrowsUp, getStackAlignment(), 0),
       TLInfo(TM, *this) {
   CallLoweringInfo.reset(new AMDGPUCallLowering(*getTargetLowering()));
   Legalizer.reset(new AMDGPULegalizerInfo(*this, TM));
 
   RegBankInfo.reset(new AMDGPURegisterBankInfo(*getRegisterInfo()));
   InstSelector.reset(new AMDGPUInstructionSelector(
-      *this, *static_cast<AMDGPURegisterBankInfo *>(RegBankInfo.get())));
+      *this, *static_cast<AMDGPURegisterBankInfo *>(RegBankInfo.get()), TM));
 }
 
 void SISubtarget::overrideSchedPolicy(MachineSchedPolicy &Policy,
                                       unsigned NumRegionInstrs) const {
   // Track register pressure so the scheduler can try to decrease
   // pressure once register usage is above the threshold defined by
   // SIRegisterInfo::getRegPressureSetLimit()
   Policy.ShouldTrackPressure = true;

llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt

 tablegen(LLVM AMDGPUGenIntrinsics.inc -gen-tgt-intrinsic)
 tablegen(LLVM AMDGPUGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM AMDGPUGenMCPseudoLowering.inc -gen-pseudo-lowering)
 tablegen(LLVM AMDGPUGenRegisterBank.inc -gen-register-bank)
 tablegen(LLVM AMDGPUGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM AMDGPUGenSearchableTables.inc -gen-searchable-tables)
 tablegen(LLVM AMDGPUGenSubtargetInfo.inc -gen-subtarget)
 
+set(LLVM_TARGET_DEFINITIONS AMDGPUGISel.td)
+tablegen(LLVM AMDGPUGenGlobalISel.inc -gen-global-isel)
+
 add_public_tablegen_target(AMDGPUCommonTableGen)
 
 add_llvm_target(AMDGPUCodeGen
   AMDGPUAliasAnalysis.cpp
   AMDGPUAlwaysInlinePass.cpp
   AMDGPUAnnotateKernelFeatures.cpp
   AMDGPUAnnotateUniformValues.cpp
   AMDGPUArgumentUsageInfo.cpp

llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.h

unsigned getRegPressureSetLimit(const MachineFunction &MF,
                                   unsigned Idx) const override;
 
   const int *getRegUnitPressureSets(unsigned RegUnit) const override;
 
   unsigned getReturnAddressReg(const MachineFunction &MF) const {
     // Not a callee saved register.
     return AMDGPU::SGPR30_SGPR31;
   }
+  const TargetRegisterClass *
+  getConstrainedRegClassForOperand(const MachineOperand &MO,
+                                 const MachineRegisterInfo &MRI) const override;
 
 private:
   void buildSpillLoadStore(MachineBasicBlock::iterator MI,
                            unsigned LoadStoreOp,
                            int Index,
                            unsigned ValueReg,
                            bool ValueIsKill,
                            unsigned ScratchRsrcReg,

llvm/trunk/lib/Target/AMDGPU/SIRegisterInfo.cpp

 //===-- SIRegisterInfo.cpp - SI Register Information ---------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
 /// SI implementation of the TargetRegisterInfo class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SIRegisterInfo.h"
+#include "AMDGPURegisterBankInfo.h"
 #include "AMDGPUSubtarget.h"
 #include "SIInstrInfo.h"
 #include "SIMachineFunctionInfo.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"

 
 const int *SIRegisterInfo::getRegUnitPressureSets(unsigned RegUnit) const {
   static const int Empty[] = { -1 };
 
   if (hasRegUnit(AMDGPU::M0, RegUnit))
     return Empty;
   return AMDGPURegisterInfo::getRegUnitPressureSets(RegUnit);
 }
+
+const TargetRegisterClass *
+SIRegisterInfo::getConstrainedRegClassForOperand(const MachineOperand &MO,
+                                         const MachineRegisterInfo &MRI) const {
+  unsigned Size = getRegSizeInBits(MO.getReg(), MRI);
+  const RegisterBank *RB = MRI.getRegBankOrNull(MO.getReg());
+  if (!RB)
+    return nullptr;
+
+  switch (Size) {
+  case 32:
+    return RB->getID() == AMDGPU::VGPRRegBankID ? &AMDGPU::VGPR_32RegClass :
+                                                  &AMDGPU::SReg_32_XM0RegClass;
+  case 64:
+    return RB->getID() == AMDGPU::VGPRRegBankID ? &AMDGPU::VReg_64RegClass :
+                                                   &AMDGPU::SReg_64_XEXECRegClass;
+  default:
+    llvm_unreachable("not implemented");
+  }
+}

llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/inst-select-or.mir

+# RUN: llc -march=amdgcn -run-pass=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s -check-prefixes=GCN
+
+--- |
+  define void @or(i32 addrspace(1)* %global0) {ret void}
+...
+---
+
+name:            or
+legalized:       true
+regBankSelected: true
+
+# GCN-LABEL: name: or
+body: |
+  bb.0:
+    liveins: $sgpr0, $sgpr1, $vgpr0, $vgpr3_vgpr4
+    ; GCN: [[SGPR0:%[0-9]+]]:sreg_32 = COPY $sgpr0
+    ; GCN: [[SGPR1:%[0-9]+]]:sreg_32 = COPY $sgpr1
+    ; GCN: [[VGPR0:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+    %0:sgpr(s32) = COPY $sgpr0
+    %1:sgpr(s32) = COPY $sgpr1
+    %2:vgpr(s32) = COPY $vgpr0
+    %3:vgpr(s64) = COPY $vgpr3_vgpr4
+    %4:sgpr(s32) = G_CONSTANT i32 1
+    %5:sgpr(s32) = G_CONSTANT i32 4096
+
+    ; or ss
+    ; GCN: [[SS:%[0-9]+]]:sreg_32_xm0 = S_OR_B32 [[SGPR0]], [[SGPR1]]
+    %6:sgpr(s32) = G_OR %0, %1
+
+    ; or vs
+    ; GCN: [[VS:%[0-9]+]]:vgpr_32 = V_OR_B32_e32 [[SS]], [[VGPR0]]
+    %7:vgpr(s32) = G_OR %2, %6
+
+    ; or sv
+    ; GCN: [[SV:%[0-9]+]]:vgpr_32 = V_OR_B32_e32 [[SS]], [[VS]]
+    %8:vgpr(s32) = G_OR %6, %7
+
+    ; or vv
+    ; GCN: [[VV:%[0-9]+]]:vgpr_32 = V_OR_B32_e32 [[SV]], [[VGPR0]]
+    %9:vgpr(s32) = G_OR %8, %2
+
+    G_STORE %9, %3 :: (store 4 into %ir.global0)
+
+...
+---