[PATCH] D63860: [MachineCSE] Improve CSE on phi node incoming value

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/CodeGen/MachineCSE.cpp:482-490
+  MachineBasicBlock *BBPhisOnly = nullptr, *BBPhisOnlySucc = nullptr;
+  for (MachineInstr &UseMI : MRI->use_nodbg_instructions(Reg)) {
+    if (!UseMI.isPHI() || (BBPhisOnly && BBPhisOnly != UseMI.getParent())) {
+      BBPhisOnly = nullptr;
+      break;
+    }
+    if (!BBPhisOnly)
----------------
I'm not sure the phis only part is needed. It would make sense to me to check the successor blocks if it's trivial (i.e. the successor only has the one predecessor here)


================
Comment at: test/CodeGen/AMDGPU/cse-phi-incoming-val.ll:13
+
+define amdgpu_ps void @mov_opt(i32 %arg, i32 inreg %arg1, i32 inreg %arg2) local_unnamed_addr #0 {
+bb:
----------------
There should probably be additional MIR tests for this. I wouldn't trust that the control flow block layout we emit know will remain constant


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63860