[PATCH] D82638: [MachineCopyPropagation] BackwardPropagatableCopy: add check for hasOverlappingMultipleDef

[Simon Wallis via Phabricator via llvm-commits]

simonwallis2 updated this revision to Diff 281554.
simonwallis2 marked 2 inline comments as done.
simonwallis2 added a comment.

•	Could you make the MIR test smaller to include minimal instructions trigger the error, so that reviewers can review it easier?
Test was 130 lines. Now 25 lines.

•	Could you add comments to describe this method?
Comments added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82638

Files:
  llvm/lib/CodeGen/MachineCopyPropagation.cpp
  llvm/test/CodeGen/ARM/mcp-dest-regs-no-dup.mir


Index: llvm/test/CodeGen/ARM/mcp-dest-regs-no-dup.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/ARM/mcp-dest-regs-no-dup.mir
@@ -0,0 +1,25 @@
+# RUN: llc -mtriple=arm-eabi -O1 -run-pass=machine-cp %s -o - 2>&1 | FileCheck %s
+--- |
+
+  @a = hidden local_unnamed_addr global i32 0, align 4
+
+  define void @h() {
+    %1 = load i32, i32* @a, align 4
+    %2 = udiv i32 %1, 45
+    ret void
+  }
+
+...
+---
+name:            h
+body:             |
+  bb.0 (%ir-block.0):
+
+    dead renamable $r9, renamable $r0 = UMULL renamable $lr, killed renamable $r0, 14 /* CC::al */, $noreg, $noreg
+
+  ; CHECK: dead renamable [[REGISTER:lr|r[0-9]+]], renamable
+  ; CHECK-NOT: [[REGISTER]],
+  ; CHECK: = UMULL
+
+    renamable $r9 = COPY killed renamable $r0
+...
Index: llvm/lib/CodeGen/MachineCopyPropagation.cpp
===================================================================
--- llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -288,6 +288,8 @@
                                           const MachineInstr &UseI,
                                           unsigned UseIdx);
   bool hasImplicitOverlap(const MachineInstr &MI, const MachineOperand &Use);
+  bool hasOverlappingMultipleDef(const MachineInstr &MI,
+                                 const MachineOperand &MODef, Register Def);
 
   /// Candidates for deletion.
   SmallSetVector<MachineInstr *, 8> MaybeDeadCopies;
@@ -461,6 +463,21 @@
   return false;
 }
 
+/// For an MI that has multiple definitions, check whether \p MI has
+/// a definition that overlaps with another of its definitions.
+/// For example, on ARM: umull   r9, r9, lr, r0
+/// The umull instruction is unpredictable unless RdHi and RdLo are different.
+bool MachineCopyPropagation::hasOverlappingMultipleDef(
+    const MachineInstr &MI, const MachineOperand &MODef, Register Def) {
+  for (const MachineOperand &MIDef : MI.defs()) {
+    if ((&MIDef != &MODef) && MIDef.isReg() &&
+        TRI->regsOverlap(Def, MIDef.getReg()))
+      return true;
+  }
+
+  return false;
+}
+
 /// Look for available copies whose destination register is used by \p MI and
 /// replace the use in \p MI with the copy's source register.
 void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
@@ -786,6 +803,9 @@
     if (hasImplicitOverlap(MI, MODef))
       continue;
 
+    if (hasOverlappingMultipleDef(MI, MODef, Def))
+      continue;
+
     LLVM_DEBUG(dbgs() << "MCP: Replacing " << printReg(MODef.getReg(), TRI)
                       << "\n     with " << printReg(Def, TRI) << "\n     in "
                       << MI << "     from " << *Copy);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82638.281554.patch
Type: text/x-patch
Size: 2688 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200729/a79b991b/attachment-0001.bin>