[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)
----------------
piotr wrote:
> arsenm wrote:
> > 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)
> Good point, I removed the condition that BB has to contain phis only. I am checking that the BB has only one successor. However, that successor might have more than one predecessor, as the pattern I am matching here is a block containing phi nodes, and such blocks typically have more than one predecessor.
I'm not sure if this should include the phi check or not. More sample cases stressing the different numbers of successors and other instructions would be good?


================
Comment at: test/CodeGen/AMDGPU/cse-phi-incoming-val.mir:42-51
+    %11:sreg_32_xm0 = S_MOV_B32 2
+    %12:vgpr_32 = V_LSHLREV_B32_e64 killed %11:sreg_32_xm0, %3:vgpr_32, implicit $exec
+    %24:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
+    %23:vreg_64 = REG_SEQUENCE killed %12:vgpr_32, %subreg.sub0, killed %24:vgpr_32, %subreg.sub1
+    %26:sgpr_32 = V_READFIRSTLANE_B32 %23.sub0:vreg_64, implicit $exec
+    %27:sgpr_32 = V_READFIRSTLANE_B32 %23.sub1:vreg_64, implicit $exec
+    %25:sreg_64 = REG_SEQUENCE %26:sgpr_32, %subreg.sub0, %27:sgpr_32, %subreg.sub1
----------------
Can you trim some of these instructions?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63860