[PATCH] D147168: [WIP] Introduce SIInstrWorklist to process instructions in moveToVALU

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I guess this is one way to do it



================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:872
   if (!SrcReg.isVirtual() || TRI->isAGPR(*MRI, SrcReg)) {
-    TII->moveToVALU(MI, MDT);
+    SIInstrWorklist worklist;
+    worklist.insert(&MI);
----------------
Should this avoid being reconstructed for each handled case?


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6154-6155
+void SIInstrWorklist::insert(MachineInstr *MI) {
+  auto itr1 = std::find(InstrList.begin(), InstrList.end(), MI);
+  if (itr1 == InstrList.end())
+    InstrList.push_back(MI);
----------------
Why did you revert from using a SetVector for this case?


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6193-6195
+void SIInstrInfo::moveToVALUimpl(SIInstrWorklist &Worklist,
+                                 MachineDominatorTree *MDT,
+                                 MachineInstr &Inst) const {
----------------
Capitalize Impl. You can also do the refactor to use the implicit function with a different typedef for the wordlist in a precommit


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6341
+    Inst.removeOperand(1);
+  } break;
+
----------------
whitespace error


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6488-6489
+    const TargetRegisterClass *NewDstRC = getDestEquivalentVGPRClass(Inst);
+    if (!NewDstRC)
+      return;
+
----------------
Don't understand why this null check was introduced 


================
Comment at: llvm/test/CodeGen/AMDGPU/control-flow-fastregalloc.ll:13-23
+; VMEM-LABEL: divergent_if_endif:
+; VMEM:         .amd_kernel_code_t
+; VMEM-NEXT:     amd_code_version_major = 1
+; VMEM-NEXT:     amd_code_version_minor = 2
+; VMEM-NEXT:     amd_machine_kind = 1
+; VMEM-NEXT:     amd_machine_version_major = 7
+; VMEM-NEXT:     amd_machine_version_minor = 0
----------------
We don't want checks for all this old style metadata. If you're going to generate the checks, use a triple with a newer default code object. Also convert tests to generated checks in a pre-commit


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147168/new/

https://reviews.llvm.org/D147168