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

Tue Sep 26 15:31:16 PDT 2017

jonpa updated this revision to Diff 116675.
jonpa added a comment.

Improved to accurately keep track of which registers are actually clobbered in the region by means of a BitVector.

> Ah, I misread the code -- if we have more than 10 instructions, you default to *not* coalese (I somehow thought it was the opposite). That seems more reasonable. The interesting question will then be how this interacts with the scheduler, i.e. how frequently it happens that unrelated code is scheduled in between, and thus breaking the coalescing. Not sure if that is a real concern ...

See table below.

> Yes, that's why I was thinking of a simple bitmap of the 16 actual GPRs, and just set a bit if this GPR is touched (no matter via which super/subreg or alias).

I did this for the 128 bit regs instead since that is what seems mostly relevant in the end.

> (B.t.w. if you do go for the one-pair check, it needs to check for one pair except 0/1, because 0/1 isn't usable for ADDR128.)

I am hoping that by using NewRC (which should be either GR128 or ADDR128), this should work as expected.

Considering other types of code with more heavy use of GR128, would we perhaps want to also give a count for a virtual register definition of a GR128? That should avoid being too optimistic when several of these overlap.

SPEC
====

                   master               patched
  lg             : 320549               318463                -2086
  stg            : 125311               124234                -1077
  lr             :  25927                26482                 +555
  lgr            : 343901               343499                 -402
  llill          :    982                  870                 -112
  st             :  51272                51343                  +71
  stfh           :   2037                 2095                  +58
  j              : 100186               100129                  -57
  c              :  10989                11045                  +56
  "Spill|Reload"   168062               165106                -2956

Number of regions / GR128 pairs clobbered

  6070 PhysClob: 0
   206 PhysClob: 1
   187 PhysClob: 2
    17 PhysClob: 3
     8 PhysClob: 4

(Never more than 4 clobbered GR128 phys reg pairs. I therefore took "3" out of the blue as a margin.)

Number of regions / MI count

  1305 MIs: 3
  2489 MIs: 4
   628 MIs: 5
   581 MIs: 6
   258 MIs: 7
   119 MIs: 8
   103 MIs: 9
    94 MIs: 10
    61 MIs: 11
   129 MIs: 12
   220 MIs: 13
    47 MIs: 14
    49 MIs: 15
    31 MIs: 16
    39 MIs: 17
    23 MIs: 18
    42 MIs: 19
    12 MIs: 20
    46 MIs: 21
    55 MIs: 22
     6 MIs: 23
    13 MIs: 24
    14 MIs: 25
    14 MIs: 26
     3 MIs: 27
     6 MIs: 28
     7 MIs: 29
     7 MIs: 30
     1 MIs: 31
    41 MIs: 32
     3 MIs: 33
     3 MIs: 34
     2 MIs: 36
     1 MIs: 37
     1 MIs: 38
     1 MIs: 40
     1 MIs: 41
     1 MIs: 42
     3 MIs: 44
     4 MIs: 45
     2 MIs: 46
     2 MIs: 47
     2 MIs: 48
     2 MIs: 49
     2 MIs: 50
     1 MIs: 51
     1 MIs: 52
     1 MIs: 55
     2 MIs: 56
     1 MIs: 63
     1 MIs: 65
     2 MIs: 67
     1 MIs: 69
     1 MIs: 70
     1 MIs: 71
     1 MIs: 73
     1 MIs: 78
     1 MIs: 103

Great majority of these cases are then as expected small regions, so I am thinking there's no real need for an instruction limit - at least not on SPEC

https://reviews.llvm.org/D37899

Files:
  include/llvm/Target/TargetRegisterInfo.h
  lib/CodeGen/RegisterCoalescer.cpp
  lib/Target/AMDGPU/SIRegisterInfo.cpp
  lib/Target/AMDGPU/SIRegisterInfo.h
  lib/Target/ARM/ARMBaseRegisterInfo.cpp
  lib/Target/ARM/ARMBaseRegisterInfo.h
  lib/Target/SystemZ/SystemZRegisterInfo.cpp
  lib/Target/SystemZ/SystemZRegisterInfo.h
  test/CodeGen/SystemZ/regalloc-GR128.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37899.116675.patch
Type: text/x-patch
Size: 9942 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170926/cb522acc/attachment-0001.bin>