[llvm] [AMDGPU] Refine GCNHazardRecognizer hasHazard() (PR #138841)

[Carl Ritson via llvm-commits]

================
@@ -441,42 +441,96 @@ using IsExpiredFn = function_ref<bool(const MachineInstr &, int WaitStates)>;
 using GetNumWaitStatesFn = function_ref<unsigned int(const MachineInstr &)>;
 
 // Search for a hazard in a block and its predecessors.
+// StateT must implement getHashValue() and isEqual().
 template <typename StateT>
 static bool
-hasHazard(StateT State,
+hasHazard(StateT InitialState,
           function_ref<HazardFnResult(StateT &, const MachineInstr &)> IsHazard,
           function_ref<void(StateT &, const MachineInstr &)> UpdateState,
-          const MachineBasicBlock *MBB,
-          MachineBasicBlock::const_reverse_instr_iterator I,
-          DenseSet<const MachineBasicBlock *> &Visited) {
-  for (auto E = MBB->instr_rend(); I != E; ++I) {
-    // No need to look at parent BUNDLE instructions.
-    if (I->isBundle())
-      continue;
-
-    switch (IsHazard(State, *I)) {
-    case HazardFound:
-      return true;
-    case HazardExpired:
-      return false;
-    default:
-      // Continue search
-      break;
+          const MachineBasicBlock *InitialMBB,
+          MachineBasicBlock::const_reverse_instr_iterator InitialI) {
+  SmallVector<std::pair<const MachineBasicBlock *, unsigned>> Worklist;
+  SmallDenseSet<std::pair<const MachineBasicBlock *, unsigned>> Visited;
+  SmallVector<std::pair<unsigned, unsigned>, 1> Collisions;
+  SmallDenseMap<unsigned, unsigned> StateHash2Idx;
+  SmallVector<StateT> States;
+
+  // States contains a vector of unique state structures.
----------------
perlfu wrote:

> you could probably reimplement `MapVector<StateT, void>`, using `MapVector::getArrayRef`
`getArrayRef` only provides access to the values, not the keys.

> Stepping back a bit, why do you want this level of indirection in the first place?
Avoiding copying for performance is the primary reason.  Because the data is backed by a `SmallVector` pointers are also not an option (will invalid).

`DenseMap` allows for alternate Key and LookupKey types specified via traits.
I have reimplemented using this mechanism; however, the actual key type needs to be a pointer to the backing `SmallVector` *and* an index into it, because traits methods are static -- if they were not static I could capture the `SmallVector` reference and keys would only need to be indexes into the array.

Overall implementation is more code, but runs with comparable performance to previous version in this PR. Slightly worse, but still net improvement against current code without this patch.



https://github.com/llvm/llvm-project/pull/138841