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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:20
 #include "SIMachineFunctionInfo.h"
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/Analysis/ValueTracking.h"
----------------
Leftover new include?


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6176
+bool SIInstrWorklist::isDeferred(MachineInstr *MI) {
+  if (DeferredList.contains(MI))
+    return true;
----------------
Can just return contains();


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6192
 
-    case AMDGPU::S_BREV_B64:
-      splitScalar64BitUnaryOp(Worklist, Inst, AMDGPU::S_BREV_B32, true);
-      Inst.eraseFromParent();
-      continue;
+  for (MachineInstr *Inst : Worklist.getDeferredList()) {
+    moveToVALUImpl(Worklist, MDT, *Inst);
----------------
Add a comment explaining what deferred instructions are 


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6341
+    Inst.removeOperand(1);
+  } break;
+
----------------
skc7 wrote:
> arsenm wrote:
> > whitespace error
> Ran clang-format before submitting the patch. Should I move the break below the curly braces?
The break should be before the curly brace but clang-format probably doesn't handle moving across a brace 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147168