[PATCH] D41098: [InlineSpiller] Fix a crash due to lack of forward progress from remat

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
Herald added subscribers: bollu, eraman, mcrosier.

This is a very ugly fix and I'm hoping someone has a better idea on how to fix this.  What's going on is we've got an instruction with more vreg uses than there are physical registers, and rematerialization appears to assume that there's always at least one physical register available in the user instruction.  At the moment, the only instructions I know of which have this problem are PATCHPOINT, STATEPOINT, and STACKMAP, but in theory, any instruction which expects to work on a lot of stack slots at once could have this problem.

(The zexts in the test case are simply a stand-in for any rematable operation which does not fold into the use.)

The only other fix I see here is to separate the spiller and remat logic entirely and expose remat as a distinct step within the register allocator.  There are some arguments in favour of this - in particular, right now the splitter is much less good at remat then the spiller is - but it's a large restructuring for what is arguably a cornercase.


Repository:
  rL LLVM

https://reviews.llvm.org/D41098

Files:
  lib/CodeGen/InlineSpiller.cpp
  test/CodeGen/X86/statepoint-live-in.ll


Index: test/CodeGen/X86/statepoint-live-in.ll
===================================================================
--- test/CodeGen/X86/statepoint-live-in.ll
+++ test/CodeGen/X86/statepoint-live-in.ll
@@ -128,6 +128,42 @@
   ret void
 }
 
+; A variant of test7 where values are not directly foldable from stack slots.
+define void @test7(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, i32 %h, i32 %i, i32 %j, i32 %k, i32 %l, i32 %m, i32 %n, i32 %o, i32 %p, i32 %q, i32 %r, i32 %s, i32 %t, i32 %u, i32 %v, i32 %w, i32 %x, i32 %y, i32 %z) gc "statepoint-example" {
+; The code for this is terrible, check simply for correctness for the moment
+; CHECK-LABEL: test7:
+; CHECK:    callq _bar
+entry:
+  %a64 = zext i32 %a to i64
+  %b64 = zext i32 %b to i64
+  %c64 = zext i32 %c to i64
+  %d64 = zext i32 %d to i64
+  %e64 = zext i32 %e to i64
+  %f64 = zext i32 %f to i64
+  %g64 = zext i32 %g to i64
+  %h64 = zext i32 %h to i64
+  %i64 = zext i32 %i to i64
+  %j64 = zext i32 %j to i64
+  %k64 = zext i32 %k to i64
+  %l64 = zext i32 %l to i64
+  %m64 = zext i32 %m to i64
+  %n64 = zext i32 %n to i64
+  %o64 = zext i32 %o to i64
+  %p64 = zext i32 %p to i64
+  %q64 = zext i32 %q to i64
+  %r64 = zext i32 %r to i64
+  %s64 = zext i32 %s to i64
+  %t64 = zext i32 %t to i64
+  %u64 = zext i32 %u to i64
+  %v64 = zext i32 %v to i64
+  %w64 = zext i32 %w to i64
+  %x64 = zext i32 %x to i64
+  %y64 = zext i32 %y to i64
+  %z64 = zext i32 %z to i64
+  %statepoint_token1 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* @bar, i32 0, i32 2, i32 0, i32 26, i64 %a64, i64 %b64, i64 %c64, i64 %d64, i64 %e64, i64 %f64, i64 %g64, i64 %h64, i64 %i64, i64 %j64, i64 %k64, i64 %l64, i64 %m64, i64 %n64, i64 %o64, i64 %p64, i64 %q64, i64 %r64, i64 %s64, i64 %t64, i64 %u64, i64 %v64, i64 %w64, i64 %x64, i64 %y64, i64 %z64)
+  ret void
+}
+
 
 ; CHECK: Ltmp0-_test1
 ; CHECK:      .byte	1
Index: lib/CodeGen/InlineSpiller.cpp
===================================================================
--- lib/CodeGen/InlineSpiller.cpp
+++ lib/CodeGen/InlineSpiller.cpp
@@ -568,6 +568,25 @@
     return true;
   }
 
+  // If the use instruction has more distinct register uses than there are
+  // physical registers, doing a remat wouldn't guarantee the result will be
+  // able to assignable.  Instead, we need to spill and fold the reload.
+  DenseMap<const TargetRegisterClass*, unsigned> Counts;
+  DenseSet<unsigned> PhysRegSeen;
+  for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
+    MachineOperand &MO = MI.getOperand(i);
+    if (!MO.isReg() || !MO.isUse())
+      continue;
+    const auto Reg = MO.getReg();
+    if (!TargetRegisterInfo::isVirtualRegister(Reg))
+      if (!PhysRegSeen.insert(Reg).second)
+        continue;
+    const auto RC = MRI.getRegClass(Reg);
+    Counts[RC]++;
+    if (Counts[RC] > RC->getNumRegs())
+      return false;
+  }
+
   // Allocate a new register for the remat.
   unsigned NewVReg = Edit->createFrom(Original);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41098.126466.patch
Type: text/x-patch
Size: 3047 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171212/f9a4893a/attachment.bin>