[PATCH] D37899: [SystemZ] Implement shouldCoalesce() to help regalloc to avoid running out of registers with GR128 regs

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Thanks for trying this suggestion!   See a couple of inline comments.

Also, I'm wondering doing less coalescing here may lead to reduced performance somewhere?   It would be good to verify at least some benchmarks to make sure we aren't too restrictive now.



================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:163
+  // regalloc running out of registers. That may happen as long as subreg
+  // liveness is turned off per default due to being compile time expensive.
+  if (SrcRC->hasSubClassEq(&SystemZ::GR128BitRegClass) &&
----------------
While turning subreg liveness  on fixes the simpler test case you have here, the even more restricted test case I've added to the bugzilla actually still fails even then.  So I'm not sure we should mention subreg liveness here as the reason why this is needed.


================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:166
+      getRegSizeInBits(*DstRC) <= 64)
+    return false;
+
----------------
Should this be symmetrical?  I.e. something like

```
  if (NewRC->hasSubClassEq(&SystemZ::GR128BitRegClass) &&
      (getRegSizeInBits(*SrcRC) <= 64 || getRegSizeInBits(*DstRC) <= 64))
    return false;

```


================
Comment at: test/CodeGen/SystemZ/regalloc-GR128.ll:6
+define i64 @test(i64 %dividend, i64 %divisor) {
+; CHECK-NOT: LLVM ERROR: ran out of registers during register allocation
+init:
----------------
Wouldn't just running llc and checking its exit code already test that there is no LLVM ERROR?


================
Comment at: test/CodeGen/SystemZ/regalloc-GR128.ll:9
+  %rem = urem i64 %dividend, %divisor
+  call void asm sideeffect "", "{r1},{r3},{r5},{r7},{r9},{r11},{r12},{r13}"(i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 %rem)
+  ret i64 %rem
----------------
Maybe better to use the more restricted test from the bugzilla.


https://reviews.llvm.org/D37899