Lower spv_bitcast in instruction lowering instead of prelegalization.

Author: s-perron

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

@@ -594,6 +594,7 @@ bool SPIRVInstructionSelector::select(MachineInstr &I) {
   bool HasDefs = I.getNumDefs() > 0;
   Register ResVReg = HasDefs ? I.getOperand(0).getReg() : Register(0);
   SPIRVType *ResType = HasDefs ? GR.getSPIRVTypeForVReg(ResVReg) : nullptr;
+  I.dump();
   assert(!HasDefs || ResType || I.getOpcode() == TargetOpcode::G_GLOBAL_VALUE ||
          I.getOpcode() == TargetOpcode::G_IMPLICIT_DEF);
   if (spvSelect(ResVReg, ResType, I)) {
@@ -3143,6 +3144,14 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
     return selectInsertElt(ResVReg, ResType, I);
   case Intrinsic::spv_gep:
     return selectGEP(ResVReg, ResType, I);
+  case Intrinsic::spv_bitcast: {
+    Register OpReg = I.getOperand(2).getReg();
+    SPIRVType *OpType =
+        OpReg.isValid() ? GR.getSPIRVTypeForVReg(OpReg) : nullptr;
+    if (!GR.isBitcastCompatible(ResType, OpType))
+      report_fatal_error("incompatible result and operand types in a bitcast");
+    return selectOpWithSrcs(ResVReg, ResType, I, {OpReg}, SPIRV::OpBitcast);
+  }
   case Intrinsic::spv_unref_global:
   case Intrinsic::spv_init_global: {
     MachineInstr *MI = MRI->getVRegDef(I.getOperand(1).getReg());

llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp

@@ -14,6 +14,7 @@
 #include "SPIRV.h"
 #include "SPIRVGlobalRegistry.h"
 #include "SPIRVSubtarget.h"
+#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/MachineInstr.h"
@@ -188,7 +189,7 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
                          LegalizeMutations::changeElementCountTo(
                              0, ElementCount::getFixed(4)))
         .lowerIf(vectorElementCountIsGreaterThan(1, 4))
-        .alwaysLegal();
+        .custom();
     getActionDefinitionsBuilder(G_CONCAT_VECTORS)
         .legalFor(allShaderVectors)
         .lower();
@@ -415,6 +416,11 @@ bool SPIRVLegalizerInfo::legalizeCustom(
   default:
     // TODO: implement legalization for other opcodes.
     return true;
+  case TargetOpcode::G_BITCAST:
+    return legalizeBitcast(Helper, MI);
+  case TargetOpcode::G_INTRINSIC:
+    return legalizeIntrinsic(Helper, MI);
+
   case TargetOpcode::G_IS_FPCLASS:
     return legalizeIsFPClass(Helper, MI, LocObserver);
   case TargetOpcode::G_ICMP: {
@@ -441,6 +447,41 @@ bool SPIRVLegalizerInfo::legalizeCustom(
   }
 }
 
+bool SPIRVLegalizerInfo::legalizeIntrinsic(LegalizerHelper &Helper,
+                                           MachineInstr &MI) const {
+  MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
+
+  auto IntrinsicID = cast<GIntrinsic>(MI).getIntrinsicID();
+  if (IntrinsicID == Intrinsic::spv_bitcast) {
+    Register DstReg = MI.getOperand(0).getReg();
+    Register SrcReg = MI.getOperand(2).getReg();
+    LLT DstTy = MRI.getType(DstReg);
+    LLT SrcTy = MRI.getType(SrcReg);
+
+    bool isLongVector = (DstTy.isVector() && DstTy.getNumElements() > 4) ||
+                        (SrcTy.isVector() && SrcTy.getNumElements() > 4);
+
+    if (isLongVector) {
+      MIRBuilder.buildBitcast(DstReg, SrcReg);
+      MI.eraseFromParent();
+    }
+    return true;
+  }
+  return true;
+}
+
+bool SPIRVLegalizerInfo::legalizeBitcast(LegalizerHelper &Helper,
+                                         MachineInstr &MI) const {
+  MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
+  Register DstReg = MI.getOperand(0).getReg();
+  Register SrcReg = MI.getOperand(1).getReg();
+  SmallVector<Register, 1> DstRegs = {DstReg};
+  MIRBuilder.buildIntrinsic(Intrinsic::spv_bitcast, DstRegs).addUse(SrcReg);
+  MI.eraseFromParent();
+  return true;
+}
+
 // Note this code was copied from LegalizerHelper::lowerISFPCLASS and adjusted
 // to ensure that all instructions created during the lowering have SPIR-V types
 // assigned to them.

llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.h

@@ -29,11 +29,15 @@ class SPIRVLegalizerInfo : public LegalizerInfo {
 public:
   bool legalizeCustom(LegalizerHelper &Helper, MachineInstr &MI,
                       LostDebugLocObserver &LocObserver) const override;
+  bool legalizeIntrinsic(LegalizerHelper &Helper,
+                         MachineInstr &MI) const override;
+
   SPIRVLegalizerInfo(const SPIRVSubtarget &ST);
 
 private:
   bool legalizeIsFPClass(LegalizerHelper &Helper, MachineInstr &MI,
                          LostDebugLocObserver &LocObserver) const;
+  bool legalizeBitcast(LegalizerHelper &Helper, MachineInstr &MI) const;
 };
 } // namespace llvm
 #endif // LLVM_LIB_TARGET_SPIRV_SPIRVMACHINELEGALIZER_H

llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp

@@ -49,7 +49,8 @@ static bool mayBeInserted(unsigned Opcode) {
   case TargetOpcode::G_CONSTANT:
   case TargetOpcode::G_UNMERGE_VALUES:
   case TargetOpcode::G_EXTRACT_VECTOR_ELT:
-  case TargetOpcode::G_BITCAST:
+  case TargetOpcode::G_INTRINSIC:
+  case TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS:
   case TargetOpcode::G_SMAX:
   case TargetOpcode::G_UMAX:
   case TargetOpcode::G_SMIN:
@@ -201,7 +202,11 @@ static bool processInstr(MachineInstr *I, MachineFunction &MF,
     }
     break;
   }
-  case TargetOpcode::G_BITCAST: {
+  case TargetOpcode::G_INTRINSIC:
+  case TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS: {
+    if (!isSpvIntrinsic(*I, Intrinsic::spv_bitcast))
+      break;
+
     for (const auto &Use : MRI.use_nodbg_instructions(ResVReg)) {
       const unsigned UseOpc = Use.getOpcode();
       assert(UseOpc == TargetOpcode::G_EXTRACT_VECTOR_ELT ||
@@ -326,13 +331,10 @@ bool SPIRVPostLegalizer::runOnMachineFunction(MachineFunction &MF) {
 
   // TODO: Move this into is own function.
   SmallVector<MachineInstr *, 8> ExtractInstrs;
-  SmallVector<MachineInstr *, 8> BitcastInstrs;
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
       if (MI.getOpcode() == TargetOpcode::G_EXTRACT_VECTOR_ELT) {
         ExtractInstrs.push_back(&MI);
-      } else if (MI.getOpcode() == TargetOpcode::G_BITCAST) {
-        BitcastInstrs.push_back(&MI);
       }
     }
   }
@@ -343,27 +345,12 @@ bool SPIRVPostLegalizer::runOnMachineFunction(MachineFunction &MF) {
     Register Vec = MI->getOperand(1).getReg();
     Register Idx = MI->getOperand(2).getReg();
 
-    auto Intr =
-        MIB.buildIntrinsic(Intrinsic::spv_extractelt, Dst, false, false);
+    auto Intr = MIB.buildIntrinsic(Intrinsic::spv_extractelt, Dst, true, false);
     Intr.addUse(Vec);
     Intr.addUse(Idx);
 
     MI->eraseFromParent();
   }
-
-  for (MachineInstr *MI : BitcastInstrs) {
-    MachineIRBuilder MIB(*MI);
-    Register Dst = MI->getOperand(0).getReg();
-    Register Src = MI->getOperand(1).getReg();
-    SPIRVType *DstType = GR->getSPIRVTypeForVReg(Dst);
-    assert(DstType && "Destination of G_BITCAST must have a type");
-    MIB.buildInstr(SPIRV::OpBitcast)
-        .addDef(Dst)
-        .addUse(GR->getSPIRVTypeID(DstType))
-        .addUse(Src);
-    MI->eraseFromParent();
-  }
-
   return true;
 }
 

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

@@ -216,29 +216,15 @@ static void buildOpBitcast(SPIRVGlobalRegistry *GR, MachineIRBuilder &MIB,
 // in https://github.com/llvm/llvm-project/pull/110270 for even more context.
 static void selectOpBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
                              MachineIRBuilder MIB) {
-  // TODO: This change will still cause failures for the machine verifier.
-  // This is done so that the G_Bitcast legalization rules can be used.
-  // We will have to add rules to legalize the intrinsic. Note that we cannot
-  // try to lower only bitcast the verify will complain about. You could cast
-  // a long vector of float to a long vector of ints. That has to be legalized
-  // but is also an invalid G_Bitcast.
-  //
-  // Could we use G_COPY? for cases where the LLT are the same? Then lowering
-  // the G_COPY could be either an OpBitcast or OpCopyObject denpending on the
-  // source and result type.
-
-  MachineRegisterInfo &MRI = MF.getRegInfo();
   SmallVector<MachineInstr *, 16> ToErase;
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
       if (MI.getOpcode() != TargetOpcode::G_BITCAST)
         continue;
-      Register DstReg = MI.getOperand(0).getReg();
-      if (MRI.getType(DstReg).isPointer()) {
-        MIB.setInsertPt(*MI.getParent(), MI);
-        buildOpBitcast(GR, MIB, DstReg, MI.getOperand(1).getReg());
-        ToErase.push_back(&MI);
-      }
+      MIB.setInsertPt(*MI.getParent(), MI);
+      buildOpBitcast(GR, MIB, MI.getOperand(0).getReg(),
+                     MI.getOperand(1).getReg());
+      ToErase.push_back(&MI);
     }
   }
   for (MachineInstr *MI : ToErase) {
@@ -255,7 +241,7 @@ static void insertBitcasts(MachineFunction &MF, SPIRVGlobalRegistry *GR,
   SmallVector<MachineInstr *, 10> ToErase;
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
-      if (!isSpvIntrinsic(MI, Intrinsic::spv_bitcast) &&
+      if (/* !isSpvIntrinsic(MI, Intrinsic::spv_bitcast) && */
           !isSpvIntrinsic(MI, Intrinsic::spv_ptrcast))
         continue;
       assert(MI.getOperand(2).isReg());