[llvm] r344492 - [TwoAddressInstructionPass] Replace subregister uses when processing tied operands

[Bjorn Pettersson via llvm-commits]

Author: bjope
Date: Mon Oct 15 01:36:03 2018
New Revision: 344492

URL: http://llvm.org/viewvc/llvm-project?rev=344492&view=rev
Log:
[TwoAddressInstructionPass] Replace subregister uses when processing tied operands

Summary:
TwoAddressInstruction pass typically rewrites
  %1:short = foo %0.sub_lo:long
as
  %1:short = COPY %0.sub_lo:long
  %1:short = foo %1:short
when having tied operands.

If there are extra un-tied operands that uses the same reg and
subreg, such as the second and third inputs to fie here:
  %1:short = fie %0.sub_lo:long, %0.sub_hi:long, %0.sub_lo:long
then there was a bug which replaced the register %0 also for
the un-tied operand, but without changing the subregister indices.
So we used to get:
  %1:short = COPY %0.sub_lo:long
  %1:short = fie %1, %1.sub_hi:short, %1.sub_lo:short
With this fix we instead get:
  %1:short = COPY %0.sub_lo:long
  %1:short = fie %1, %0.sub_hi:long, %1

Reviewers: arsenm, JesperAntonsson, kparzysz, MatzeB

Reviewed By: MatzeB

Subscribers: bjope, kparzysz, wdng, llvm-commits

Differential Revision: https://reviews.llvm.org/D36224

Added:
    llvm/trunk/test/CodeGen/Hexagon/two-addr-tied-subregs.mir
Modified:
    llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp

Modified: llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp?rev=344492&r1=344491&r2=344492&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp (original)
+++ llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp Mon Oct 15 01:36:03 2018
@@ -1608,23 +1608,28 @@ TwoAddressInstructionPass::processTiedPa
   }
 
   if (AllUsesCopied) {
+    bool ReplacedAllUntiedUses = true;
     if (!IsEarlyClobber) {
       // Replace other (un-tied) uses of regB with LastCopiedReg.
       for (MachineOperand &MO : MI->operands()) {
-        if (MO.isReg() && MO.getReg() == RegB &&
-            MO.isUse()) {
-          if (MO.isKill()) {
-            MO.setIsKill(false);
-            RemovedKillFlag = true;
+        if (MO.isReg() && MO.getReg() == RegB && MO.isUse()) {
+          if (MO.getSubReg() == SubRegB) {
+            if (MO.isKill()) {
+              MO.setIsKill(false);
+              RemovedKillFlag = true;
+            }
+            MO.setReg(LastCopiedReg);
+            MO.setSubReg(0);
+          } else {
+            ReplacedAllUntiedUses = false;
           }
-          MO.setReg(LastCopiedReg);
-          MO.setSubReg(MO.getSubReg());
         }
       }
     }
 
     // Update live variables for regB.
-    if (RemovedKillFlag && LV && LV->getVarInfo(RegB).removeKill(*MI)) {
+    if (RemovedKillFlag && ReplacedAllUntiedUses &&
+        LV && LV->getVarInfo(RegB).removeKill(*MI)) {
       MachineBasicBlock::iterator PrevMI = MI;
       --PrevMI;
       LV->addVirtualRegisterKilled(RegB, *PrevMI);

Added: llvm/trunk/test/CodeGen/Hexagon/two-addr-tied-subregs.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/two-addr-tied-subregs.mir?rev=344492&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/two-addr-tied-subregs.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/two-addr-tied-subregs.mir Mon Oct 15 01:36:03 2018
@@ -0,0 +1,56 @@
+# RUN: llc -march hexagon -run-pass livevars -run-pass twoaddressinstruction  -verify-machineinstrs -o - %s | FileCheck %s
+
+
+###############################################################################
+
+---
+name:            test1
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $d0
+
+    %0:doubleregs = COPY killed $d0
+    %1:intregs = S2_lsr_i_r_acc %0.isub_lo, %0.isub_lo, 16
+
+...
+
+# Verify that both uses if %0.isub_lo are replaced here.
+# (we used to get %1:intregs = S2_lsr_i_r_acc %1, %1.isub_lo, 16)
+#
+# CHECK-LABEL: name:            test1
+# CHECK:  bb.0.entry:
+# CHECK:      %0:doubleregs = COPY killed $d0
+# CHECK-NEXT: %1:intregs = COPY killed %0.isub_lo
+# CHECK-NEXT: %1:intregs = S2_lsr_i_r_acc %1, %1, 16
+
+
+###############################################################################
+
+---
+name:            test2
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $d0
+
+    %0:doubleregs = COPY killed $d0
+    %1:intregs = S2_lsr_i_r_acc %0.isub_lo, %0.isub_hi, 16
+
+...
+
+# Verify that the use of %0.isub_hi isn't replaced here.
+# (we used to get %1:intregs = S2_lsr_i_r_acc %1, %1.isub_hi, 16)
+#
+# We also used to get an incorrect "killed" for %0 in the second COPY.
+# So we verify that we do not get machine verifier complaints here.
+# An improvement could be to get a "killed" attribute on the last
+# use of %0.isub_hi, but we do not need it for the IR to be valid.
+#
+# CHECK-LABEL: name:            test2
+# CHECK:  bb.0.entry:
+# CHECK:      %0:doubleregs = COPY killed $d0
+# CHECK-NEXT: %1:intregs = COPY %0.isub_lo
+# CHECK-NEXT: %1:intregs = S2_lsr_i_r_acc %1, %0.isub_hi, 16
+
+###############################################################################