[llvm] r330018 - [PostRASink]Add register dependency check for implicit operands

Jun Bum Lim via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 13 07:23:09 PDT 2018


Author: junbuml
Date: Fri Apr 13 07:23:09 2018
New Revision: 330018

URL: http://llvm.org/viewvc/llvm-project?rev=330018&view=rev
Log:
[PostRASink]Add register dependency check for implicit operands

Summary:
This change extend the register dependency check for implicit operands in Copy instructions.
Fixes PR36902.

Reviewers: thegameg, sebpop, uweigand, jnspaulsson, gberry, mcrosier, qcolombet, MatzeB

Reviewed By: thegameg

Subscribers: llvm-commits

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

Added:
    llvm/trunk/test/CodeGen/SystemZ/no-postra-sink.mir
Modified:
    llvm/trunk/lib/CodeGen/MachineSink.cpp

Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=330018&r1=330017&r2=330018&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineSink.cpp Fri Apr 13 07:23:09 2018
@@ -955,7 +955,7 @@ public:
 
   MachineFunctionProperties getRequiredProperties() const override {
     return MachineFunctionProperties().set(
-      MachineFunctionProperties::Property::NoVRegs);
+        MachineFunctionProperties::Property::NoVRegs);
   }
 
 private:
@@ -975,6 +975,14 @@ char &llvm::PostRAMachineSinkingID = Pos
 INITIALIZE_PASS(PostRAMachineSinking, "postra-machine-sink",
                 "PostRA Machine Sink", false, false)
 
+static bool aliasWithRegsInLiveIn(MachineBasicBlock *SI,
+                                  SmallSet<unsigned, 8> &AliasedRegs) {
+  for (const auto LI : SI->liveins())
+    if (AliasedRegs.count(LI.PhysReg))
+      return true;
+  return false;
+}
+
 static MachineBasicBlock *
 getSingleLiveInSuccBB(MachineBasicBlock &CurBB,
                       ArrayRef<MachineBasicBlock *> SinkableBBs, unsigned Reg,
@@ -1002,13 +1010,92 @@ getSingleLiveInSuccBB(MachineBasicBlock
   for (auto *SI : CurBB.successors()) {
     if (SI == BB)
       continue;
-    for (const auto LI : SI->liveins())
-      if (AliasedRegs.count(LI.PhysReg))
-        return nullptr;
+    if (aliasWithRegsInLiveIn(SI, AliasedRegs))
+      return nullptr;
   }
   return BB;
 }
 
+static MachineBasicBlock *getSingleLiveInSuccBB(
+    MachineBasicBlock &CurBB, ArrayRef<MachineBasicBlock *> SinkableBBs,
+    ArrayRef<unsigned> DefedRegsInCopy, const TargetRegisterInfo *TRI) {
+  MachineBasicBlock *SingleBB = nullptr;
+  for (auto DefReg : DefedRegsInCopy) {
+    MachineBasicBlock *BB =
+        getSingleLiveInSuccBB(CurBB, SinkableBBs, DefReg, TRI);
+    if (!BB || (SingleBB && SingleBB != BB))
+      return nullptr;
+    SingleBB = BB;
+  }
+  return SingleBB;
+}
+
+static void clearKillFlags(MachineInstr *MI, MachineBasicBlock &CurBB,
+                           SmallVectorImpl<unsigned> &UsedOpsInCopy,
+                           BitVector &UsedRegs, const TargetRegisterInfo *TRI) {
+  for (auto U : UsedOpsInCopy) {
+    MachineOperand &MO = MI->getOperand(U);
+    unsigned SrcReg = MO.getReg();
+    if (UsedRegs[SrcReg]) {
+      MachineBasicBlock::iterator NI = std::next(MI->getIterator());
+      for (MachineInstr &UI : make_range(NI, CurBB.end())) {
+        if (UI.killsRegister(SrcReg, TRI)) {
+          UI.clearRegisterKills(SrcReg, TRI);
+          MO.setIsKill(true);
+          break;
+        }
+      }
+    }
+  }
+}
+
+static void updateLiveIn(MachineInstr *MI, MachineBasicBlock *SuccBB,
+                         SmallVectorImpl<unsigned> &UsedOpsInCopy,
+                         SmallVectorImpl<unsigned> &DefedRegsInCopy) {
+  for (auto DefReg : DefedRegsInCopy)
+    SuccBB->removeLiveIn(DefReg);
+  for (auto U : UsedOpsInCopy) {
+    unsigned Reg = MI->getOperand(U).getReg();
+    if (!SuccBB->isLiveIn(Reg))
+      SuccBB->addLiveIn(Reg);
+  }
+}
+
+static bool hasRegisterDependency(MachineInstr *MI,
+                                  SmallVectorImpl<unsigned> &UsedOpsInCopy,
+                                  SmallVectorImpl<unsigned> &DefedRegsInCopy,
+                                  BitVector &ModifiedRegs,
+                                  BitVector &UsedRegs) {
+  bool HasRegDependency = false;
+  for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
+    MachineOperand &MO = MI->getOperand(i);
+    if (!MO.isReg())
+      continue;
+    unsigned Reg = MO.getReg();
+    if (!Reg)
+      continue;
+    if (MO.isDef()) {
+      if (ModifiedRegs[Reg] || UsedRegs[Reg]) {
+        HasRegDependency = true;
+        break;
+      }
+      DefedRegsInCopy.push_back(Reg);
+
+      // FIXME: instead of isUse(), readsReg() would be a better fix here,
+      // For example, we can ignore modifications in reg with undef. However,
+      // it's not perfectly clear if skipping the internal read is safe in all
+      // other targets.
+    } else if (MO.isUse()) {
+      if (ModifiedRegs[Reg]) {
+        HasRegDependency = true;
+        break;
+      }
+      UsedOpsInCopy.push_back(i);
+    }
+  }
+  return HasRegDependency;
+}
+
 bool PostRAMachineSinking::tryToSinkCopy(MachineBasicBlock &CurBB,
                                          MachineFunction &MF,
                                          const TargetRegisterInfo *TRI,
@@ -1044,16 +1131,21 @@ bool PostRAMachineSinking::tryToSinkCopy
       continue;
     }
 
-    unsigned DefReg = MI->getOperand(0).getReg();
-    unsigned SrcReg = MI->getOperand(1).getReg();
+    // Track the operand index for use in Copy.
+    SmallVector<unsigned, 2> UsedOpsInCopy;
+    // Track the register number defed in Copy.
+    SmallVector<unsigned, 2> DefedRegsInCopy;
+
     // Don't sink the COPY if it would violate a register dependency.
-    if (ModifiedRegs[DefReg] || ModifiedRegs[SrcReg] || UsedRegs[DefReg]) {
+    if (hasRegisterDependency(MI, UsedOpsInCopy, DefedRegsInCopy, ModifiedRegs,
+                              UsedRegs)) {
       TII->trackRegDefsUses(*MI, ModifiedRegs, UsedRegs, TRI);
       continue;
     }
-
+    assert((!UsedOpsInCopy.empty() && !DefedRegsInCopy.empty()) &&
+           "Unexpect SrcReg or DefReg");
     MachineBasicBlock *SuccBB =
-        getSingleLiveInSuccBB(CurBB, SinkableBBs, DefReg, TRI);
+        getSingleLiveInSuccBB(CurBB, SinkableBBs, DefedRegsInCopy, TRI);
     // Don't sink if we cannot find a single sinkable successor in which Reg
     // is live-in.
     if (!SuccBB) {
@@ -1065,22 +1157,10 @@ bool PostRAMachineSinking::tryToSinkCopy
 
     // Clear the kill flag if SrcReg is killed between MI and the end of the
     // block.
-    if (UsedRegs[SrcReg]) {
-      MachineBasicBlock::iterator NI = std::next(MI->getIterator());
-      for (MachineInstr &UI : make_range(NI, CurBB.end())) {
-        if (UI.killsRegister(SrcReg, TRI)) {
-          UI.clearRegisterKills(SrcReg, TRI);
-          MI->getOperand(1).setIsKill(true);
-          break;
-        }
-      }
-    }
-
+    clearKillFlags(MI, CurBB, UsedOpsInCopy, UsedRegs, TRI);
     MachineBasicBlock::iterator InsertPos = SuccBB->getFirstNonPHI();
     SuccBB->splice(InsertPos, &CurBB, MI);
-    SuccBB->removeLiveIn(DefReg);
-    if (!SuccBB->isLiveIn(SrcReg))
-      SuccBB->addLiveIn(SrcReg);
+    updateLiveIn(MI, SuccBB, UsedOpsInCopy, DefedRegsInCopy);
 
     Changed = true;
     ++NumPostRACopySink;

Added: llvm/trunk/test/CodeGen/SystemZ/no-postra-sink.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/no-postra-sink.mir?rev=330018&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/SystemZ/no-postra-sink.mir (added)
+++ llvm/trunk/test/CodeGen/SystemZ/no-postra-sink.mir Fri Apr 13 07:23:09 2018
@@ -0,0 +1,63 @@
+# RUN: llc -mtriple=s390x-linux-gnu -mcpu=z13 -run-pass=postra-machine-sink -verify-machineinstrs -o - %s  | FileCheck %s
+
+---
+# Don't sink COPY to bb.2 since SLLK define r13l that is aliased with r12q.
+# CHECK-LABEL: name: donot_sink_copy
+# CHECK-LABEL: bb.0:
+# CHECK: renamable $r0l = COPY renamable $r12l, implicit killed $r12q
+# CHECK-LABEL: bb.2:
+# CHECK-NOT: COPY
+name:            donot_sink_copy
+tracksRegLiveness: true
+body:             |
+  bb.0 :
+    successors: %bb.1, %bb.2
+    liveins: $r2d, $r3d, $r4d, $r5d, $r12q
+
+    renamable $r0l = COPY renamable $r12l, implicit killed $r12q
+    renamable $r13l = SLLK renamable $r4l, $noreg, 1
+    CHIMux renamable $r3l, 0, implicit-def $cc, implicit killed $r3d
+    BRC 14, 6, %bb.2, implicit killed $cc
+    J %bb.1
+
+  bb.1:
+    successors:
+
+  bb.2:
+    successors:
+    liveins: $r2d, $r4d, $r5d, $r0l, $r13l
+
+    renamable $r0d = LGFR killed renamable $r0l
+    renamable $r11d = LGFR killed renamable $r13l
+...
+
+# Don't sink COPY to bb.2 since SLLK use r1l that is aliased with r0q.
+# CHECK-LABEL: name: donot_sink_copy2
+# CHECK-LABEL: bb.0:
+# CHECK: renamable $r0l = COPY renamable $r12l, implicit-def $r0q
+# CHECK-LABEL: bb.2:
+# CHECK-NOT: COPY
+name:            donot_sink_copy2
+tracksRegLiveness: true
+body:             |
+  bb.0 :
+    successors: %bb.1, %bb.2
+    liveins: $r2d, $r3d, $r4d, $r5d, $r12q
+
+    renamable $r0l = COPY renamable $r12l, implicit def $r0q
+    renamable $r13l = SLLK renamable $r1l, $noreg, 1
+    CHIMux renamable $r3l, 0, implicit-def $cc, implicit killed $r3d
+    BRC 14, 6, %bb.2, implicit killed $cc
+    J %bb.1
+
+  bb.1:
+    successors:
+
+  bb.2:
+    successors:
+    liveins: $r2d, $r4d, $r5d, $r0l, $r13l
+
+    renamable $r0d = LGFR killed renamable $r0l
+    renamable $r11d = LGFR killed renamable $r13l
+...
+




More information about the llvm-commits mailing list