[PATCH] D54839: [CodeGen] Enhance machine PHIs optimization

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked 7 inline comments as done.
anton-afanasyev added inline comments.


================
Comment at: lib/CodeGen/OptimizePHIs.cpp:123-125
+      if (MI->getOperand(i+1).getMBB() == SrcMI->getParent()) {
+        // If the copy instr is in the predecessor BB (almost always for PHI
+        // reg operands), try to propagate copy's operand.
----------------
MatzeB wrote:
> I don't undestand why COPYs would be more likely in the predecessor blocks of the PHIs (this is before PHI lowering after all). And it also doesn't make any sense to me to have different behavior depending on where the COPY is, can't we just always look through the COPY and change `SrcReg`?
Yes, you are right, this check is redundant! I'm to remove it.


================
Comment at: test/CodeGen/X86/opt_phis2.mir:15-35
+  - { id: 0, class: vr256, preferred-register: '' }
+  - { id: 1, class: vr256, preferred-register: '' }
+  - { id: 2, class: vr256, preferred-register: '' }
+  - { id: 3, class: vr256, preferred-register: '' }
+  - { id: 4, class: vr256, preferred-register: '' }
+  - { id: 5, class: vr256, preferred-register: '' }
+  - { id: 6, class: vr256, preferred-register: '' }
----------------
MatzeB wrote:
> Things should also work without this block, as you already define all the register classes on the MIR operands themselfes.
Yes, thank you!


================
Comment at: test/CodeGen/X86/opt_phis2.mir:36-39
+liveins:
+  - { reg: '$edi', virtual-reg: '%7' }
+  - { reg: '$ymm0', virtual-reg: '%8' }
+  - { reg: '$rsi', virtual-reg: '%9' }
----------------
MatzeB wrote:
> I suspect you can simply drop this block as well for this test
Ok.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54839