Fix removal of dead elements from PressureDiff

[ygribov]

Summary

Fix removal of dead elements from PressureDiff. This fixes https://bugs.llvm.org/show_bug.cgi?id=37252

I'm nervous that this might introduce some scheduling churn which will cause unrelated fails on bots.

[MatzeB]

Good catch, the fix looks good to me. I assume you built llvm will all (non-experimental) targets and tested them?

Independently of this patch it is worrying that this requires test changes. That means X86 schedules produce intermediate pressure sets with MaxPSets/16 different pressure sets inside...

[ygribov]

Good catch, the fix looks good to me. I assume you built llvm will all (non-experimental) targets and tested them?

Yes, I ran cmake with default settings so all default cores were built. One caveat is that testing was done on Windows (~27K tests, done according to instructions in https://llvm.org/docs/GettingStartedVS.html).

Independently of this patch it is worrying that this requires test changes. That means X86 schedules produce intermediate pressure sets with MaxPSets/16 different pressure sets inside...

Yes, here's what I'm getting for e.g. divrem8_ext.ll:

-		PressureChanges	0x0000000002663400 {{PSetID=2 UnitInc=1 }, {PSetID=11 UnitInc=1 }, {PSetID=12 UnitInc=1 }, {PSetID=13 ...}, ...}	llvm::PressureChange[16]
+		[0]	{PSetID=2 UnitInc=1 }	llvm::PressureChange      GR8_ABCD_L
+		[1]	{PSetID=11 UnitInc=1 }	llvm::PressureChange      GR32_TC
+		[2]	{PSetID=12 UnitInc=1 }	llvm::PressureChange      LOW32_ADDR_ACCESS_with_sub_32bit+GR64_NOREX_and_GR64_TCW64
+		[3]	{PSetID=13 UnitInc=1 }	llvm::PressureChange      GR64_NOREX_and_GR64_TC
+		[4]	{PSetID=14 UnitInc=1 }	llvm::PressureChange      LOW32_ADDR_ACCESS_with_sub_32bit+GR64_NOREX_and_GR64_TC
+		[5]	{PSetID=18 UnitInc=1 }	llvm::PressureChange      GR64_NOREX
+		[6]	{PSetID=19 UnitInc=1 }	llvm::PressureChange      GR64_TCW64
+		[7]	{PSetID=20 UnitInc=1 }	llvm::PressureChange      LOW32_ADDR_ACCESS_with_sub_32bit+GR64_TCW64
+		[8]	{PSetID=21 UnitInc=1 }	llvm::PressureChange      GR64_TC
+		[9]	{PSetID=22 UnitInc=1 }	llvm::PressureChange      LOW32_ADDR_ACCESS_with_sub_32bit+GR64_TC
+		[10]	{PSetID=23 UnitInc=1 }	llvm::PressureChange      GR64_TC+GR64_TCW64
+		[11]	{PSetID=25 UnitInc=-1 }	llvm::PressureChange      GR8+GR64_NOREX
+		[12]	{PSetID=26 UnitInc=-1 }	llvm::PressureChange      GR8+GR64_TCW64
+		[13]	{PSetID=28 UnitInc=-1 }	llvm::PressureChange      GR8+GR64_TC
+		[14]	{PSetID=30 UnitInc=-1 }	llvm::PressureChange      GR16
+		[15]	{PSetID=30 UnitInc=-1 }	llvm::PressureChange      GR16

As a side note the current limit of 16 elements may be too strict for some targets (e.g. it caused a lot of noise on our non-orthogonal architecture until I increased it significantly) so it may make sense to switch to dynamic array.

[ygribov]

Reopening (commit caused lit regressions).

[ygribov]

Meanwhile few more tests required changes.

llvm/trunk/lib/CodeGen/RegisterPressure.cpp

for (; PSetI.isValid(); ++PSetI) {
     unsigned NewUnitInc = I->getUnitInc() + Weight;
     if (NewUnitInc != 0) {
       I->setUnitInc(NewUnitInc);
     } else {
       // Remove entry
       PressureDiff::iterator J;
       for (J = std::next(I); J != E && J->isValid(); ++J, ++I)
         *I = *J;
-      if (J != E)
-        *I = *J;
+      *I = PressureChange();
     }
   }
 }
 
 /// Force liveness of registers.
 void RegPressureTracker::addLiveRegs(ArrayRef<RegisterMaskPair> Regs) {
   for (const RegisterMaskPair &P : Regs) {
     LaneBitmask PrevMask = LiveRegs.insert(P);

llvm/trunk/test/CodeGen/X86/GlobalISel/gep.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL --check-prefix=X64_GISEL
 ; RUN: llc -mtriple=x86_64-linux-gnu              -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL --check-prefix=X64
 
 define i32* @test_gep_i8(i32 *%arr, i8 %ind) {
 ; X64_GISEL-LABEL: test_gep_i8:
 ; X64_GISEL:       # %bb.0:
 ; X64_GISEL-NEXT:    # kill: def $esi killed $esi def $rsi
-; X64_GISEL-NEXT:    movq $4, %rax
 ; X64_GISEL-NEXT:    movq $56, %rcx
 ; X64_GISEL-NEXT:    # kill: def $cl killed $rcx
 ; X64_GISEL-NEXT:    shlq %cl, %rsi
 ; X64_GISEL-NEXT:    movq $56, %rcx
 ; X64_GISEL-NEXT:    # kill: def $cl killed $rcx
+; X64_GISEL-NEXT:    movq $4, %rax
 ; X64_GISEL-NEXT:    sarq %cl, %rsi
 ; X64_GISEL-NEXT:    imulq %rax, %rsi
 ; X64_GISEL-NEXT:    leaq (%rdi,%rsi), %rax
 ; X64_GISEL-NEXT:    retq
 ;
 ; X64-LABEL: test_gep_i8:
 ; X64:       # %bb.0:
 ; X64-NEXT:    # kill: def $esi killed $esi def $rsi
; X64-NEXT:    retq
   %arrayidx = getelementptr i32, i32* %arr, i8 20
   ret i32* %arrayidx
 }
 
 define i32* @test_gep_i16(i32 *%arr, i16 %ind) {
 ; X64_GISEL-LABEL: test_gep_i16:
 ; X64_GISEL:       # %bb.0:
 ; X64_GISEL-NEXT:    # kill: def $esi killed $esi def $rsi
-; X64_GISEL-NEXT:    movq $4, %rax
 ; X64_GISEL-NEXT:    movq $48, %rcx
 ; X64_GISEL-NEXT:    # kill: def $cl killed $rcx
 ; X64_GISEL-NEXT:    shlq %cl, %rsi
 ; X64_GISEL-NEXT:    movq $48, %rcx
 ; X64_GISEL-NEXT:    # kill: def $cl killed $rcx
+; X64_GISEL-NEXT:    movq $4, %rax
 ; X64_GISEL-NEXT:    sarq %cl, %rsi
 ; X64_GISEL-NEXT:    imulq %rax, %rsi
 ; X64_GISEL-NEXT:    leaq (%rdi,%rsi), %rax
 ; X64_GISEL-NEXT:    retq
 ;
 ; X64-LABEL: test_gep_i16:
 ; X64:       # %bb.0:
 ; X64-NEXT:    # kill: def $esi killed $esi def $rsi

llvm/trunk/test/CodeGen/X86/divrem.ll

; X64-NEXT:    retq
   store i64 %t, i64* %q
   ret void
 }
 
 define void @si32(i32 %x, i32 %y, i32* %p, i32* %q) nounwind {
 ; X32-LABEL: si32:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %esi
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    cltd
 ; X32-NEXT:    idivl {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movl %eax, (%esi)
 ; X32-NEXT:    movl %edx, (%ecx)
 ; X32-NEXT:    popl %esi
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: si32:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movq %rdx, %r8
; X64-NEXT:    retq
   store i32 %t, i32* %q
   ret void
 }
 
 define void @si16(i16 %x, i16 %y, i16* %p, i16* %q) nounwind {
 ; X32-LABEL: si16:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %esi
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movzwl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    cwtd
 ; X32-NEXT:    idivw {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movw %ax, (%esi)
 ; X32-NEXT:    movw %dx, (%ecx)
 ; X32-NEXT:    popl %esi
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: si16:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movq %rdx, %r8
; X64-NEXT:    retq
   store i16 %t, i16* %q
   ret void
 }
 
 define void @si8(i8 %x, i8 %y, i8* %p, i8* %q) nounwind {
 ; X32-LABEL: si8:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %ebx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:    movb {{[0-9]+}}(%esp), %al
 ; X32-NEXT:    cbtw
 ; X32-NEXT:    idivb {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:    movsbl %ah, %ebx
 ; X32-NEXT:    movb %al, (%edx)
 ; X32-NEXT:    movb %bl, (%ecx)
 ; X32-NEXT:    popl %ebx
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: si8:
 ; X64:       # %bb.0:
; X64-NEXT:    retq
   store i64 %t, i64* %q
   ret void
 }
 
 define void @ui32(i32 %x, i32 %y, i32* %p, i32* %q) nounwind {
 ; X32-LABEL: ui32:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %esi
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    xorl %edx, %edx
 ; X32-NEXT:    divl {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movl %eax, (%esi)
 ; X32-NEXT:    movl %edx, (%ecx)
 ; X32-NEXT:    popl %esi
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: ui32:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movq %rdx, %r8
; X64-NEXT:    retq
   store i32 %t, i32* %q
   ret void
 }
 
 define void @ui16(i16 %x, i16 %y, i16* %p, i16* %q) nounwind {
 ; X32-LABEL: ui16:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %esi
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movzwl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    xorl %edx, %edx
 ; X32-NEXT:    divw {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:    movw %ax, (%esi)
 ; X32-NEXT:    movw %dx, (%ecx)
 ; X32-NEXT:    popl %esi
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: ui16:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movq %rdx, %r8
; X64-NEXT:    retq
   store i16 %t, i16* %q
   ret void
 }
 
 define void @ui8(i8 %x, i8 %y, i8* %p, i8* %q) nounwind {
 ; X32-LABEL: ui8:
 ; X32:       # %bb.0:
 ; X32-NEXT:    pushl %ebx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:    movzbl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    # kill: def $eax killed $eax def $ax
 ; X32-NEXT:    divb {{[0-9]+}}(%esp)
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:    movzbl %ah, %ebx
 ; X32-NEXT:    movb %al, (%edx)
 ; X32-NEXT:    movb %bl, (%ecx)
 ; X32-NEXT:    popl %ebx
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: ui8:
 ; X64:       # %bb.0:

llvm/trunk/test/CodeGen/X86/divrem8_ext.ll

; X64-NEXT:    retq
   %1 = srem i8 %x, %y
   ret i8 %1
 }
 
 define i8 @test_srem_noext_ah(i8 %x, i8 %y) {
 ; X32-LABEL: test_srem_noext_ah:
 ; X32:       # %bb.0:
 ; X32-NEXT:    movb {{[0-9]+}}(%esp), %al
-; X32-NEXT:    movb {{[0-9]+}}(%esp), %cl
 ; X32-NEXT:    cbtw
+; X32-NEXT:    movb {{[0-9]+}}(%esp), %cl
 ; X32-NEXT:    idivb %cl
 ; X32-NEXT:    movsbl %ah, %eax
 ; X32-NEXT:    addb %cl, %al
 ; X32-NEXT:    # kill: def $al killed $al killed $eax
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: test_srem_noext_ah:
 ; X64:       # %bb.0: