[PATCH] D55867: [RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: MatzeB, qcolombet.
Herald added a subscriber: tpr.

This is to fix the bug in PR40061 related with https://reviews.llvm.org/rL339035.

In https://reviews.llvm.org/rL339035, live interval of source pseudo register in rematerialized copy is saved in ToBeUpdated set and its update is postponed.

In PR40061, %t2 = %t1 is rematerialized and %t1 is added into toBeUpdated set to postpone its live interval update. After the rematerialization, the live interval of %t1 is larger than necessary. Then %t1 is merged into %t3 and %t1 gets removed. After the merge, %t3 contains live interval larger than necessary. Because %t3 is not in toBeUpdated set, its live interval is not updated after register coalescing and it will break some assumption in regalloc.

The patch requires the live interval of destination register in a merge to be updated if the source register is in ToBeUpdated.


Repository:
  rL LLVM

https://reviews.llvm.org/D55867

Files:
  lib/CodeGen/RegisterCoalescer.cpp
  test/CodeGen/X86/late-remat-update-2.mir


Index: test/CodeGen/X86/late-remat-update-2.mir
===================================================================
--- test/CodeGen/X86/late-remat-update-2.mir
+++ test/CodeGen/X86/late-remat-update-2.mir
@@ -0,0 +1,61 @@
+# REQUIRES: asserts
+# RUN: llc -mtriple=x86_64-- -run-pass=simple-register-coalescing -late-remat-update-threshold=0 -debug-only=regalloc %s -o /dev/null 2>&1 | FileCheck %s
+#
+# PR40061: %t2 = %t1 is rematerialized and %t1 is added into toBeUpdated set
+# to postpone its live interval update. After the rematerialization, the live
+# interval of %t1 is larger than necessary. Then %t1 is merged into %t3 and %t1
+# gets removed. After the merge, %t3 contains live interval larger than
+# necessary. Because %t3 is not in toBeUpdated set so its live interval is not
+# updated after register coalescing, and it will break some assumption in
+# regalloc. This test checks the live interval is up-to-date after register
+# coalescing.
+#
+# Check the interval of %t3 after %t1 is merged into %t3 in register coalescing.
+# Make sure the interval of %t3 doesn't cover the entry of bb.1 because %t1 is
+# rematerialized in bb.1.
+# CHECK: INTERVALS
+# CHECK: %2 [16r,80B:1)[240B,272r:1)[272r,304r:0)  0 at 272r 1 at 16r
+#
+# CHECK: MACHINEINSTRS
+# Check %t1 has been merged with %t3
+# CHECK: 0B	bb.0.entry:
+# CHECK: 16B	  %t3:gr64 = MOV64ri32 -11
+# CHECK: 32B	  CMP64ri8 %t3:gr64, 1
+# Check %t1 here has been rematerialized.
+# CHECK: 80B	bb.1:
+# CHECK: 96B	  %t2:gr64 = MOV64ri32 -11
+
+---
+name:            _Z3fooi
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x15555555), %bb.2(0x6aaaaaab)
+
+    %t1:gr64 = MOV64ri32 -11
+    CMP64ri8 %t1, 1, implicit-def $eflags
+    JE_1 %bb.2, implicit killed $eflags
+    JMP_1 %bb.1
+
+  bb.1:
+    successors: %bb.1(0x80000000)
+
+    %t2:gr64 = COPY %t1
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %t2:gr64 = ADD64ri8 %t2, 5, implicit-def $eflags
+    $rdi = COPY %t2
+    CALL64pcrel32 @_Z3fooi, csr_64, implicit $rsp, implicit $ssp, implicit killed $rdi, implicit-def $rsp, implicit-def $ssp
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CMP64ri8 %t2, 1, implicit-def $eflags
+    JE_1 %bb.1, implicit killed $eflags
+    RET -1
+
+  bb.2:
+    successors: %bb.3(0x80000000)
+    %t3:gr64 = COPY %t1
+    %t3:gr64 = ADD64ri8 %t3, 10, implicit-def $eflags
+
+  bb.3:
+    $rax = COPY %t3
+    RET 0, $rax
+
+...
Index: lib/CodeGen/RegisterCoalescer.cpp
===================================================================
--- lib/CodeGen/RegisterCoalescer.cpp
+++ lib/CodeGen/RegisterCoalescer.cpp
@@ -1910,6 +1910,14 @@
     }
     LI.removeEmptySubRanges();
   }
+
+  // CP.getSrcReg()'s live interval is merged into CP.getDstReg's live
+  // interval here. Since CP.getSrcReg() is in ToBeUpdated set and its
+  // live interval is not up-to-date, need to update CP.getDstReg's live
+  // interval here.
+  if (ToBeUpdated.count(CP.getSrcReg()))
+    ShrinkMainRange = true;
+
   if (ShrinkMainRange) {
     LiveInterval &LI = LIS->getInterval(CP.getDstReg());
     shrinkToUses(&LI);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55867.178812.patch
Type: text/x-patch
Size: 3260 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181219/0e8d0f6b/attachment.bin>