harden implementation of the duplicate tracker; fix generation of global variables

Author: VyacheslavLevytskyy

llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp

@@ -536,9 +536,8 @@ static Register buildBuiltinVariableLoad(
 /// SPIRVType in ASSIGN_TYPE, otherwise create it from \p Ty. Defined in
 /// SPIRVPreLegalizer.cpp.
 extern void insertAssignInstr(Register Reg, Type *Ty, SPIRVType *SpirvTy,
-                                  SPIRVGlobalRegistry *GR,
-                                  MachineIRBuilder &MIB,
-                                  MachineRegisterInfo &MRI);
+                              SPIRVGlobalRegistry *GR, MachineIRBuilder &MIB,
+                              MachineRegisterInfo &MRI);
 
 // TODO: Move to TableGen.
 static SPIRV::MemorySemantics::MemorySemantics

llvm/lib/Target/SPIRV/SPIRVIRMapping.h

@@ -1,4 +1,4 @@
-//===-- SPIRVDuplicatesTracker.h - SPIR-V Duplicates Tracker ----*- C++ -*-===//
+//===------------ SPIRVMapping.h - SPIR-V Duplicates Tracker ----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -26,28 +26,6 @@
 
 namespace llvm {
 namespace SPIRV {
-/*
-inline size_t to_hash(const MachineInstr *MI,
-                      std::unordered_set<const MachineInstr *> &Visited) {
-  if (!MI || !Visited.insert(MI).second)
-    return 0;
-  const MachineRegisterInfo &MRI = MI->getMF()->getRegInfo();
-  SmallVector<size_t, 16> Codes{MI->getOpcode()};
-  size_t H;
-  for (unsigned I = MI->getNumDefs(); I < MI->getNumOperands(); ++I) {
-    const MachineOperand &MO = MI->getOperand(I);
-    H = MO.isReg() ? to_hash(getDef(MO, &MRI), Visited)
-                   : size_t(llvm::hash_value(MO));
-    Codes.push_back(H);
-  }
-  return llvm::hash_combine(Codes.begin(), Codes.end());
-}
-
-inline size_t to_hash(const MachineInstr *MI) {
-  std::unordered_set<const MachineInstr *> Visited;
-  return to_hash(MI, Visited);
-}
-*/
 
 inline size_t to_hash(const MachineInstr *MI) {
   hash_code H = llvm::hash_combine(MI->getOpcode(), MI->getNumOperands());
@@ -63,10 +41,10 @@ inline size_t to_hash(const MachineInstr *MI) {
   return H;
 }
 
-using MIHandle = std::pair<const MachineInstr *, size_t>;
+using MIHandle = std::tuple<const MachineInstr *, Register, size_t>;
 
 inline MIHandle getMIKey(const MachineInstr *MI) {
-  return std::make_pair(MI, SPIRV::to_hash(MI));
+  return std::make_tuple(MI, MI->getOperand(0).getReg(), SPIRV::to_hash(MI));
 }
 
 using IRHandle = std::tuple<const void *, unsigned, unsigned>;
@@ -200,10 +178,8 @@ class SPIRVIRMapping {
     SPIRV::MIHandle MIKey = SPIRV::getMIKey(MI);
     auto It1 = Vregs.try_emplace(HandleMF, MIKey);
     if (!It1.second) {
-      // there is an expired record
-      auto [ExistMI, _] = It1.first->second;
-      // invalidate the record
-      Defs.erase(ExistMI);
+      // there is an expired record that we need to invalidate
+      Defs.erase(std::get<0>(It1.first->second));
       // update the record
       It1.first->second = MIKey;
     }
@@ -225,13 +201,14 @@ class SPIRVIRMapping {
     auto It = Vregs.find(HandleMF);
     if (It == Vregs.end())
       return nullptr;
-    auto [MI, Hash] = It->second;
-    assert(SPIRV::to_hash(MI) == Hash);
-    assert(Defs.find(MI) != Defs.end() && Defs.find(MI)->second == HandleMF);
-    /*if (SPIRV::to_hash(MI) != Hash) {
+    auto [MI, Reg, Hash] = It->second;
+    const MachineInstr *Def = MF->getRegInfo().getVRegDef(Reg);
+    if (!Def || Def != MI || SPIRV::to_hash(MI) != Hash) {
+      // there is an expired record that we need to invalidate
       erase(MI);
       return nullptr;
-    }*/
+    }
+    assert(Defs.find(MI) != Defs.end() && Defs.find(MI)->second == HandleMF);
     return MI;
   }
   Register find(SPIRV::IRHandle Handle, const MachineFunction *MF) {

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

@@ -1041,7 +1041,8 @@ bool SPIRVInstructionSelector::selectUnOp(Register ResVReg,
     for (MachineRegisterInfo::def_instr_iterator DefIt =
              MRI->def_instr_begin(SrcReg);
          DefIt != MRI->def_instr_end(); DefIt = std::next(DefIt)) {
-      if ((*DefIt).getOpcode() == TargetOpcode::G_GLOBAL_VALUE) {
+      if ((*DefIt).getOpcode() == TargetOpcode::G_GLOBAL_VALUE ||
+          (*DefIt).getOpcode() == SPIRV::OpVariable) {
         IsGV = true;
         break;
       }
@@ -2727,7 +2728,6 @@ bool SPIRVInstructionSelector::selectOpUndef(Register ResVReg,
       .constrainAllUses(TII, TRI, RBI);
 }
 
-
 bool SPIRVInstructionSelector::selectInsertVal(Register ResVReg,
                                                const SPIRVType *ResType,
                                                MachineInstr &I) const {
@@ -2886,7 +2886,16 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
                              ? MRI->getVRegDef(I.getOperand(2).getReg())
                              : nullptr;
     assert(MI);
-    return selectGlobalValue(MI->getOperand(0).getReg(), *MI, Init);
+    Register GVarVReg = MI->getOperand(0).getReg();
+    bool Res = selectGlobalValue(GVarVReg, *MI, Init);
+    // We violate SSA form by inserting OpVariable having a gMIR instruction
+    // %vreg = G_GLOBAL_VALUE @gvar
+    // We need to fix this erasing the duplicated definition.
+    if (MI->getOpcode() == TargetOpcode::G_GLOBAL_VALUE) {
+      GR.invalidateMachineInstr(MI);
+      MI->removeFromParent();
+    }
+    return Res;
   }
   case Intrinsic::spv_undef: {
     auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpUndef))