[X86] Correct spill slot size.

This patch fixes a bug introduced by revision 330778 (originally reviewed at: https://reviews.llvm.org/D44782), where function isFrameLoadOpcode returned the wrong number of bytes read for opcodes VMOVSSrm and VMOVSDrm. This corrects that mistake, and extends the regression test to catch cases where the dead stores should be removed. Patch by Jeremy Morse. Differential Revision: https://reviews.llvm.org/D46256 llvm-svn: 331252
llvm · May 1, 2018 · d4c5840 · d4c5840
1 parent 0537fdb
commit d4c5840
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 2 deletions.
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -3954,13 +3954,14 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
   case X86::MOV32rm:
   case X86::MOVSSrm:
   case X86::VMOVSSZrm:
+  case X86::VMOVSSrm:
   case X86::KMOVDkm:
     MemBytes = 4;
     return true;
   case X86::MOV64rm:
   case X86::LD_Fp64m:
   case X86::MOVSDrm:
-  case X86::VMOVSSrm:
+  case X86::VMOVSDrm:
   case X86::VMOVSDZrm:
   case X86::MMX_MOVD64rm:
   case X86::MMX_MOVQ64rm:
@@ -3973,7 +3974,6 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
   case X86::MOVUPDrm:
   case X86::MOVDQArm:
   case X86::MOVDQUrm:
-  case X86::VMOVSDrm:
   case X86::VMOVAPSrm:
   case X86::VMOVUPSrm:
   case X86::VMOVAPDrm:

diff --git a/llvm/test/CodeGen/X86/pr30821.mir b/llvm/test/CodeGen/X86/pr30821.mir
@@ -111,6 +111,74 @@ body:             |
 
     MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %2 :: (volatile dereferenceable store 16 into %ir.india)
 
+
+    ; Test some sequences that _should_ be eliminated
+    %3:vr128 = MOVUPDrm %stack.2.india, 1, $noreg, 0, $noreg :: (volatile dereferenceable load 16 from %ir.india)
+
+    %32:fr64 = VMOVSDrm %stack.2.india, 1, $noreg, 0, $noreg :: (dereferenceable load 8 from %ir.india)
+    %33:fr64 = COPY killed %32
+    VMOVSDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %33 :: (store 8 into %ir.india)
+
+    ; This is the spill introduced by regalloc; we check that the inner dead
+    ; store and load were eliminated
+    ; CHECK: MOVAPSmr %stack.3, 1, $noreg, 0, $noreg, killed renamable $xmm{{[0-9]+}} :: (store 16 into %stack.3)
+    ; CHECK-NEXT:renamable $xmm{{[0-9]+}} = MOVAPSrm %stack.3, 1, $noreg, 0, $noreg :: (load 16 from %stack.3)
+
+    MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %3 :: (volatile dereferenceable store 16 into %ir.india)
+
+
+    ; Moves with different encodings but same size should be eliminated
+    %4:vr128 = MOVUPDrm %stack.2.india, 1, $noreg, 0, $noreg :: (volatile dereferenceable load 16 from %ir.india)
+
+    %42:fr32 = MOVSSrm %stack.2.india, 1, $noreg, 0, $noreg :: (dereferenceable load 4 from %ir.india)
+    %43:fr32 = COPY killed %42
+    VMOVSSZmr %stack.2.india, 1, $noreg, 0, $noreg, killed %43 :: (store 4 into %ir.india)
+
+    ; CHECK: MOVAPSmr %stack.3, 1, $noreg, 0, $noreg, killed renamable $xmm{{[0-9]+}} :: (store 16 into %stack.3)
+    ; CHECK-NEXT:renamable $xmm{{[0-9]+}} = MOVAPSrm %stack.3, 1, $noreg, 0, $noreg :: (load 16 from %stack.3)
+
+    MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %4 :: (volatile dereferenceable store 16 into %ir.india)
+
+
+    ; Same deal with double-size
+    %5:vr128 = MOVUPDrm %stack.2.india, 1, $noreg, 0, $noreg :: (volatile dereferenceable load 16 from %ir.india)
+
+    %52:fr64 = MOVSDrm %stack.2.india, 1, $noreg, 0, $noreg :: (dereferenceable load 8 from %ir.india)
+    %53:fr64 = COPY killed %52
+    VMOVSDZmr %stack.2.india, 1, $noreg, 0, $noreg, killed %53 :: (store 8 into %ir.india)
+
+    ; CHECK: MOVAPSmr %stack.3, 1, $noreg, 0, $noreg, killed renamable $xmm{{[0-9]+}} :: (store 16 into %stack.3)
+    ; CHECK-NEXT:renamable $xmm{{[0-9]+}} = MOVAPSrm %stack.3, 1, $noreg, 0, $noreg :: (load 16 from %stack.3)
+
+    MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %5 :: (volatile dereferenceable store 16 into %ir.india)
+
+
+    ; Last two repeated, with load/store opcode flipped
+    %6:vr128 = MOVUPDrm %stack.2.india, 1, $noreg, 0, $noreg :: (volatile dereferenceable load 16 from %ir.india)
+
+    %62:fr32 = VMOVSSZrm %stack.2.india, 1, $noreg, 0, $noreg :: (dereferenceable load 4 from %ir.india)
+    %63:fr32 = COPY killed %62
+    MOVSSmr %stack.2.india, 1, $noreg, 0, $noreg, killed %63 :: (store 4 into %ir.india)
+
+    ; CHECK: MOVAPSmr %stack.3, 1, $noreg, 0, $noreg, killed renamable $xmm{{[0-9]+}} :: (store 16 into %stack.3)
+    ; CHECK-NEXT:renamable $xmm{{[0-9]+}} = MOVAPSrm %stack.3, 1, $noreg, 0, $noreg :: (load 16 from %stack.3)
+
+    MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %6 :: (volatile dereferenceable store 16 into %ir.india)
+
+
+    ; Flipped double-size different-encoding test
+    %7:vr128 = MOVUPDrm %stack.2.india, 1, $noreg, 0, $noreg :: (volatile dereferenceable load 16 from %ir.india)
+
+    %72:fr64 = VMOVSDZrm %stack.2.india, 1, $noreg, 0, $noreg :: (dereferenceable load 8 from %ir.india)
+    %73:fr64 = COPY killed %72
+    MOVSDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %73 :: (store 8 into %ir.india)
+
+    ; CHECK: MOVAPSmr %stack.3, 1, $noreg, 0, $noreg, killed renamable $xmm{{[0-9]+}} :: (store 16 into %stack.3)
+    ; CHECK-NEXT:renamable $xmm{{[0-9]+}} = MOVAPSrm %stack.3, 1, $noreg, 0, $noreg :: (load 16 from %stack.3)
+
+    MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed %7 :: (volatile dereferenceable store 16 into %ir.india)
+
+
     ; Stores of first 15 $xmm registers to keep them live across the middle of
     ; this bb.
     MOVUPDmr %stack.2.india, 1, $noreg, 0, $noreg, killed $xmm0 :: (volatile dereferenceable store 16 into %ir.india)