[Spill2Reg][8/9] Added code generation support for 8/16bit spills/reloads in x86

[craig.topper]

llvm/test/CodeGen/X86/spill2reg_end_to_end_8bit.ll

71

This code looks invalid. movb can't have a %eax destination

[vporpo]

llvm/test/CodeGen/X86/spill2reg_end_to_end_8bit.ll

71

Yes, this patch needs more work. I need to update it.

[vporpo]

Refactored the patch and fixed illegal instructions.

[craig.topper]

llvm/lib/Target/X86/X86InstrInfo.cpp

9587

Should use the AVX or AVX512 opcodes when available?

Are you limiting to VR128 regclass or do you allow VR128X with AVX512?

llvm/test/CodeGen/X86/spill2reg_end_to_end_16bit.ll

70–71

Why is X86FixupBWInsts.cpp failing to convert this load to movzwl?

[vporpo]

llvm/lib/Target/X86/X86InstrInfo.cpp

9587

I am trying to avoid the frequency throttling associated with using wide vector instructions. But I guess AVX should be safe ? Not sure about AVX512 though, won't it cause frequency drops if I use that instead?
I am currently using VR128.

llvm/test/CodeGen/X86/spill2reg_end_to_end_16bit.ll

70–71

XF86FixupBWInsts.cpp:208 checks whether the super reg %eax is live after movw D0(%rip), %ax. In the original code it is not live, but in our case it is because movd is reading from %eax. So it bails and does not replace the register.
Hmm not sure how we could fix this though.

[vporpo]

Removed the subregs from the movd instructions.
Also added simple mir tests for 8/16 bits.

[vporpo]

Updated tests.

[vporpo]

Rebased.

[Carrot]

llvm/lib/Target/X86/X86InstrInfo.cpp

9636

Does it cause partial write whole read hazard?

[vporpo]

llvm/lib/Target/X86/X86InstrInfo.cpp

9636

It might, though I don't know what the overhead is. Do you know how I could
measure it? This code shows up in the test CodeGen/X86/spill2reg_end_to_end_8bit.ll but it runs ~7% faster with spill2reg enabled on a skylake machine, but perhaps this is not a good test to evaluate it.

Do you think it is worth adding a flag that disables it by default?

llvm/lib/Target/X86/X86InstrInfo.h

private:
 
   const TargetRegisterClass *getVectorRegisterClassForSpill2Reg(
       const TargetRegisterInfo *TRI) const override;
 
   bool isSpill2RegProfitable(const MachineInstr *MI,
                              const TargetRegisterInfo *TRI,
                              const MachineRegisterInfo *MRI) const override;
 
+  llvm::Optional<MCRegister>
+  getMovdCompatibleReg(MCRegister OldReg, uint32_t OldRegBits,
+                       const TargetRegisterInfo *TRI) const;
+
   MachineInstr *
   spill2RegInsertToVectorReg(Register DstReg, Register SrcReg,
                              int OperationBits, MachineBasicBlock *MBB,
                              MachineBasicBlock::iterator InsertBeforeIt,
                              const TargetRegisterInfo *TRI) const override;
 
   MachineInstr *
   spill2RegExtractFromVectorReg(Register DstReg, Register SrcReg,
                                 int OperationBits, MachineBasicBlock *InsertMBB,
                                 MachineBasicBlock::iterator InsertBeforeIt,
                                 const TargetRegisterInfo *TRI) const override;
 };
 
 } // namespace llvm
 
 #endif

llvm/lib/Target/X86/X86InstrInfo.cpp

bool X86InstrInfo::isLegalToSpill2Reg(unsigned MemBits, unsigned Opcode) const {
   case X86::KMOVDmk:
   case X86::KMOVQkm:
   case X86::KMOVQmk:
     return false;
   }
   switch (MemBits) {
   case 64:
   case 32:
+  case 16:
+  case 8:
     return true;
   }
   return false;
 }
 
 bool X86InstrInfo::targetSupportsSpill2Reg(
     const TargetSubtargetInfo *STI) const {
   const X86Subtarget *X86STI = static_cast<const X86Subtarget *>(STI);
bool X86InstrInfo::isSpill2RegProfitable(const MachineInstr *MI,
   bool MemHeuristic =
       Spill2RegMemInstrsThreshold == 0 || CntMem >= Spill2RegMemInstrsThreshold;
   bool VecHeuristic = CntVec <= Spill2RegVecInstrsThreshold;
   return MemHeuristic && VecHeuristic;
 }
 
 static unsigned getInsertOrExtractOpcode(unsigned Bits, bool Insert) {
   switch (Bits) {
+  case 8:
+  case 16:
   case 32:
     return Insert ? X86::MOVDI2PDIrr : X86::MOVPDI2DIrr;
   case 64:
     return Insert ? X86::MOV64toPQIrr : X86::MOVPQIto64rr;
   default:
     llvm_unreachable("Unsupported bits");
   }
 }
 
+/// \Returns the subreg index for a getting a subregister of \p SubregBits from
+/// a register of \p RegBits.
+static unsigned spill2RegGetSubregIdx(unsigned RegBits, unsigned SubregBits) {
+  assert(RegBits > SubregBits && "From expected to cover To");
+  switch (SubregBits) {
+  case 32:
+    return X86::sub_32bit;
+  case 16:
+    return X86::sub_16bit;
+  case 8:
+    return X86::sub_8bit;
+  default:
+    llvm_unreachable("FIXME");
+  }
+}
+
+llvm::Optional<MCRegister>
+X86InstrInfo::getMovdCompatibleReg(MCRegister OldReg, uint32_t OldRegBits,
+                                   const TargetRegisterInfo *TRI) const {
+  if (OldRegBits != 8 && OldRegBits != 16)
+    return None;
+  // The register class of the register that movd can handle.
+  const TargetRegisterClass *NewRegClass =
+      TRI->getRegClass(X86::GR32RegClassID);
+  unsigned NewRegBits = TRI->getRegSizeInBits(*NewRegClass);
+  unsigned SubIdx = spill2RegGetSubregIdx(NewRegBits, OldRegBits);
+  MCRegister NewReg = TRI->getMatchingSuperReg(OldReg, SubIdx, NewRegClass);
+  return NewReg;
+}
+
 MachineInstr *X86InstrInfo::spill2RegInsertToVectorReg(
     Register DstReg, Register SrcReg, int OperationBits, MachineBasicBlock *MBB,
     MachineBasicBlock::iterator InsertBeforeIt,
     const TargetRegisterInfo *TRI) const {
   DebugLoc DL;
   unsigned InsertOpcode =
       getInsertOrExtractOpcode(OperationBits, true /*insert*/);
   const MCInstrDesc &InsertMCID = get(InsertOpcode);
+  // `movd` does not support 8/16 bit operands. Instead, we use a 32-bit
+  // register. For example:
+  //   $al = ...
+  //   $xmm0 = MOVPDI2DIrr $eax
+  if (auto NewReg = getMovdCompatibleReg(SrcReg, OperationBits, TRI))
+    SrcReg = *NewReg;
   MachineInstr *InsertMI =
       BuildMI(*MBB, InsertBeforeIt, DL, InsertMCID, DstReg).addReg(SrcReg);
   InsertMI->addRegisterKilled(DstReg, TRI);
   return InsertMI;
 }
 
 MachineInstr *X86InstrInfo::spill2RegExtractFromVectorReg(
     Register DstReg, Register SrcReg, int OperationBits,
     MachineBasicBlock *InsertMBB, MachineBasicBlock::iterator InsertBeforeIt,
     const TargetRegisterInfo *TRI) const {
   DebugLoc DL;
   unsigned ExtractOpcode =
       getInsertOrExtractOpcode(OperationBits, false /*extract*/);
   const MCInstrDesc &ExtractMCID = get(ExtractOpcode);
+  // `movd` does not support 8/16 bit operands. Instead, we use a 32-bit
+  // register. For example:
+  //   $eax = MOVPDI2DIrr $xmm0
+  //   ... = $al
+  if (auto NewReg = getMovdCompatibleReg(DstReg, OperationBits, TRI))
+    DstReg = *NewReg;
   MachineInstr *ExtractMI =
       BuildMI(*InsertMBB, InsertBeforeIt, DL, ExtractMCID, DstReg)
           .addReg(SrcReg);
   ExtractMI->addRegisterKilled(DstReg, TRI);
   return ExtractMI;
 }
 
 #define GET_INSTRINFO_HELPERS
 #include "X86GenInstrInfo.inc"

llvm/test/CodeGen/X86/spill2reg_end_to_end_16bit.ll

 ; CHECK-NEXT:    pushq %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 56
 ; CHECK-NEXT:    .cfi_offset %rbx, -56
 ; CHECK-NEXT:    .cfi_offset %r12, -48
 ; CHECK-NEXT:    .cfi_offset %r13, -40
 ; CHECK-NEXT:    .cfi_offset %r14, -32
 ; CHECK-NEXT:    .cfi_offset %r15, -24
 ; CHECK-NEXT:    .cfi_offset %rbp, -16
-; CHECK-NEXT:    movzwl D0(%rip), %eax
-; CHECK-NEXT:    movw %ax, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
+; CHECK-NEXT:    movw D0(%rip), %ax
+; CHECK-NEXT:    movd %eax, %xmm3
 ; CHECK-NEXT:    movzwl D1(%rip), %ecx
 ; CHECK-NEXT:    movzwl D2(%rip), %edx
 ; CHECK-NEXT:    movzwl D3(%rip), %esi
 ; CHECK-NEXT:    movzwl D4(%rip), %edi
 ; CHECK-NEXT:    movzwl D5(%rip), %r8d
 ; CHECK-NEXT:    movzwl D6(%rip), %r9d
 ; CHECK-NEXT:    movzwl D7(%rip), %r10d
 ; CHECK-NEXT:    movzwl D8(%rip), %r11d
 ; CHECK-NEXT:    movzwl D9(%rip), %ebx
 ; CHECK-NEXT:    movzwl D10(%rip), %ebp
 ; CHECK-NEXT:    movzwl D11(%rip), %r14d
 ; CHECK-NEXT:    movzwl D12(%rip), %r15d
 ; CHECK-NEXT:    movzwl D13(%rip), %r12d
 ; CHECK-NEXT:    movzwl D14(%rip), %r13d
-; CHECK-NEXT:    movzwl D15(%rip), %eax
-; CHECK-NEXT:    movw %ax, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
-; CHECK-NEXT:    movzwl D16(%rip), %eax
-; CHECK-NEXT:    movw %ax, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
-; CHECK-NEXT:    movzwl D17(%rip), %eax
-; CHECK-NEXT:    movw %ax, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
-; CHECK-NEXT:    movzwl D18(%rip), %eax
-; CHECK-NEXT:    movw %ax, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
+; CHECK-NEXT:    movw D15(%rip), %ax
+; CHECK-NEXT:    movd %eax, %xmm0
+; CHECK-NEXT:    movw D16(%rip), %ax
+; CHECK-NEXT:    movd %eax, %xmm1
+; CHECK-NEXT:    movw D17(%rip), %ax
+; CHECK-NEXT:    movd %eax, %xmm4
+; CHECK-NEXT:    movw D18(%rip), %ax
+; CHECK-NEXT:    movd %eax, %xmm2
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    #NO_APP
-; CHECK-NEXT:    movzwl {{[-0-9]+}}(%r{{[sb]}}p), %eax # 2-byte Folded Reload
+; CHECK-NEXT:    movd %xmm3, %eax
 ; CHECK-NEXT:    movw %ax, U0(%rip)
 ; CHECK-NEXT:    movw %cx, U1(%rip)
 ; CHECK-NEXT:    movw %dx, U2(%rip)
 ; CHECK-NEXT:    movw %si, U3(%rip)
 ; CHECK-NEXT:    movw %di, U4(%rip)
 ; CHECK-NEXT:    movw %r8w, U5(%rip)
 ; CHECK-NEXT:    movw %r9w, U6(%rip)
 ; CHECK-NEXT:    movw %r10w, U7(%rip)
 ; CHECK-NEXT:    movw %r11w, U8(%rip)
 ; CHECK-NEXT:    movw %bx, U9(%rip)
 ; CHECK-NEXT:    movw %bp, U10(%rip)
 ; CHECK-NEXT:    movw %r14w, U11(%rip)
 ; CHECK-NEXT:    movw %r15w, U12(%rip)
 ; CHECK-NEXT:    movw %r12w, U13(%rip)
 ; CHECK-NEXT:    movw %r13w, U14(%rip)
-; CHECK-NEXT:    movzwl {{[-0-9]+}}(%r{{[sb]}}p), %eax # 2-byte Folded Reload
+; CHECK-NEXT:    movd %xmm0, %eax
 ; CHECK-NEXT:    movw %ax, U15(%rip)
-; CHECK-NEXT:    movzwl {{[-0-9]+}}(%r{{[sb]}}p), %eax # 2-byte Folded Reload
+; CHECK-NEXT:    movd %xmm1, %eax
 ; CHECK-NEXT:    movw %ax, U16(%rip)
-; CHECK-NEXT:    movzwl {{[-0-9]+}}(%r{{[sb]}}p), %eax # 2-byte Folded Reload
+; CHECK-NEXT:    movd %xmm4, %eax
 ; CHECK-NEXT:    movw %ax, U17(%rip)
-; CHECK-NEXT:    movzwl {{[-0-9]+}}(%r{{[sb]}}p), %eax # 2-byte Folded Reload
+; CHECK-NEXT:    movd %xmm2, %eax
 ; CHECK-NEXT:    movw %ax, U18(%rip)
 ; CHECK-NEXT:    popq %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 48
 ; CHECK-NEXT:    popq %r12
 ; CHECK-NEXT:    .cfi_def_cfa_offset 40
 ; CHECK-NEXT:    popq %r13
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    popq %r14

llvm/test/CodeGen/X86/spill2reg_end_to_end_8bit.ll

 ; CHECK-NEXT:    .cfi_def_cfa_offset 56
 ; CHECK-NEXT:    .cfi_offset %rbx, -56
 ; CHECK-NEXT:    .cfi_offset %r12, -48
 ; CHECK-NEXT:    .cfi_offset %r13, -40
 ; CHECK-NEXT:    .cfi_offset %r14, -32
 ; CHECK-NEXT:    .cfi_offset %r15, -24
 ; CHECK-NEXT:    .cfi_offset %rbp, -16
 ; CHECK-NEXT:    movb D0(%rip), %al
-; CHECK-NEXT:    movb %al, {{[-0-9]+}}(%r{{[sb]}}p) # 1-byte Spill
+; CHECK-NEXT:    movd %eax, %xmm3
 ; CHECK-NEXT:    movb D1(%rip), %cl
 ; CHECK-NEXT:    movb D2(%rip), %dl
 ; CHECK-NEXT:    movb D3(%rip), %sil
 ; CHECK-NEXT:    movb D4(%rip), %dil
 ; CHECK-NEXT:    movb D5(%rip), %r8b
 ; CHECK-NEXT:    movb D6(%rip), %r9b
 ; CHECK-NEXT:    movb D7(%rip), %r10b
 ; CHECK-NEXT:    movb D8(%rip), %r11b
 ; CHECK-NEXT:    movb D9(%rip), %bl
 ; CHECK-NEXT:    movb D10(%rip), %bpl
 ; CHECK-NEXT:    movb D11(%rip), %r14b
 ; CHECK-NEXT:    movb D12(%rip), %r15b
 ; CHECK-NEXT:    movb D13(%rip), %r12b
 ; CHECK-NEXT:    movb D14(%rip), %r13b
 ; CHECK-NEXT:    movb D15(%rip), %al
-; CHECK-NEXT:    movb %al, {{[-0-9]+}}(%r{{[sb]}}p) # 1-byte Spill
+; CHECK-NEXT:    movd %eax, %xmm0
 ; CHECK-NEXT:    movb D16(%rip), %al
-; CHECK-NEXT:    movb %al, {{[-0-9]+}}(%r{{[sb]}}p) # 1-byte Spill
+; CHECK-NEXT:    movd %eax, %xmm1
 ; CHECK-NEXT:    movb D17(%rip), %al
-; CHECK-NEXT:    movb %al, {{[-0-9]+}}(%r{{[sb]}}p) # 1-byte Spill
+; CHECK-NEXT:    movd %eax, %xmm4
 ; CHECK-NEXT:    movb D18(%rip), %al
-; CHECK-NEXT:    movb %al, {{[-0-9]+}}(%r{{[sb]}}p) # 1-byte Spill
+; CHECK-NEXT:    movd %eax, %xmm2
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    #NO_APP
-; CHECK-NEXT:    movb {{[-0-9]+}}(%r{{[sb]}}p), %al # 1-byte Reload
+; CHECK-NEXT:    movd %xmm3, %eax
 ; CHECK-NEXT:    movb %al, U0(%rip)
 ; CHECK-NEXT:    movb %cl, U1(%rip)
 ; CHECK-NEXT:    movb %dl, U2(%rip)
 ; CHECK-NEXT:    movb %sil, U3(%rip)
 ; CHECK-NEXT:    movb %dil, U4(%rip)
 ; CHECK-NEXT:    movb %r8b, U5(%rip)
 ; CHECK-NEXT:    movb %r9b, U6(%rip)
 ; CHECK-NEXT:    movb %r10b, U7(%rip)
 ; CHECK-NEXT:    movb %r11b, U8(%rip)
 ; CHECK-NEXT:    movb %bl, U9(%rip)
 ; CHECK-NEXT:    movb %bpl, U10(%rip)
 ; CHECK-NEXT:    movb %r14b, U11(%rip)
 ; CHECK-NEXT:    movb %r15b, U12(%rip)
 ; CHECK-NEXT:    movb %r12b, U13(%rip)
 ; CHECK-NEXT:    movb %r13b, U14(%rip)
-; CHECK-NEXT:    movb {{[-0-9]+}}(%r{{[sb]}}p), %al # 1-byte Reload
+; CHECK-NEXT:    movd %xmm0, %eax
 ; CHECK-NEXT:    movb %al, U15(%rip)
-; CHECK-NEXT:    movb {{[-0-9]+}}(%r{{[sb]}}p), %al # 1-byte Reload
+; CHECK-NEXT:    movd %xmm1, %eax
 ; CHECK-NEXT:    movb %al, U16(%rip)
-; CHECK-NEXT:    movb {{[-0-9]+}}(%r{{[sb]}}p), %al # 1-byte Reload
+; CHECK-NEXT:    movd %xmm4, %eax
 ; CHECK-NEXT:    movb %al, U17(%rip)
-; CHECK-NEXT:    movb {{[-0-9]+}}(%r{{[sb]}}p), %al # 1-byte Reload
+; CHECK-NEXT:    movd %xmm2, %eax
 ; CHECK-NEXT:    movb %al, U18(%rip)
 ; CHECK-NEXT:    popq %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 48
 ; CHECK-NEXT:    popq %r12
 ; CHECK-NEXT:    .cfi_def_cfa_offset 40
 ; CHECK-NEXT:    popq %r13
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    popq %r14

llvm/test/CodeGen/X86/spill2reg_simple_1_16bit.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc %s -o - -mtriple=x86_64-unknown-linux -enable-spill2reg -mattr=+sse4.1 --run-pass=spill2reg -simplify-mir -spill2reg-mem-instrs=0 -spill2reg-vec-instrs=99999 | FileCheck %s
+
+# Simple test with a single 16-bit spill-reload pair:
+#   spill stack.0
+#   reload stack.0
+
+--- |
+  @D0 = dso_local local_unnamed_addr global i32 0, align 4
+  @U0 = dso_local local_unnamed_addr global i32 0, align 4
+  define void @func() { ret void }
+...
+---
+name: func
+alignment:       16
+tracksRegLiveness: true
+tracksDebugUserValues: true
+frameInfo:
+  maxAlignment:    4
+stack:
+  - { id: 0, type: spill-slot, size: 4, alignment: 4 }
+machineFunctionInfo: {}
+body:             |
+
+  bb.0:
+    ; spill
+    ; CHECK-LABEL: name: func
+    ; CHECK: $ax = MOV16rm $rip, 1, $noreg, @D0, $noreg :: (dereferenceable load (s16) from @D0)
+    ; CHECK-NEXT: $xmm0 = MOVDI2PDIrr $eax
+    ; CHECK-NEXT: $eax = MOVPDI2DIrr $xmm0
+    ; CHECK-NEXT: MOV16mr $rip, 1, $noreg, @U0, $noreg, killed renamable $ax :: (store (s16) into @U0)
+    ; CHECK-NEXT: RET 0
+    $ax = MOV16rm $rip, 1, $noreg, @D0, $noreg :: (dereferenceable load (s16) from @D0)
+    MOV16mr %stack.0, 1, $noreg, 0, $noreg, killed renamable $ax :: (store (s16) into %stack.0)
+    ; reload
+    $ax = MOV16rm %stack.0, 1, $noreg, 0, $noreg :: (load (s16) from %stack.0)
+    MOV16mr $rip, 1, $noreg, @U0, $noreg, killed renamable $ax :: (store (s16) into @U0)
+    RET 0
+...

llvm/test/CodeGen/X86/spill2reg_simple_1_8bit.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc %s -o - -mtriple=x86_64-unknown-linux -enable-spill2reg -mattr=+sse4.1 --run-pass=spill2reg -simplify-mir -spill2reg-mem-instrs=0 -spill2reg-vec-instrs=99999 | FileCheck %s
+
+# Simple test with a single 8-bit spill-reload pair:
+#   spill stack.0
+#   reload stack.0
+
+--- |
+  @D0 = dso_local local_unnamed_addr global i32 0, align 4
+  @U0 = dso_local local_unnamed_addr global i32 0, align 4
+  define void @func() { ret void }
+...
+---
+name: func
+alignment:       16
+tracksRegLiveness: true
+tracksDebugUserValues: true
+frameInfo:
+  maxAlignment:    4
+stack:
+  - { id: 0, type: spill-slot, size: 4, alignment: 4 }
+machineFunctionInfo: {}
+body:             |
+
+  bb.0:
+    ; spill
+    ; CHECK-LABEL: name: func
+    ; CHECK: $al = MOV8rm $rip, 1, $noreg, @D0, $noreg :: (dereferenceable load (s8) from @D0)
+    ; CHECK-NEXT: $xmm0 = MOVDI2PDIrr $eax
+    ; CHECK-NEXT: $eax = MOVPDI2DIrr $xmm0
+    ; CHECK-NEXT: MOV8mr $rip, 1, $noreg, @U0, $noreg, killed renamable $al :: (store (s8) into @U0)
+    ; CHECK-NEXT: RET 0
+    $al = MOV8rm $rip, 1, $noreg, @D0, $noreg :: (dereferenceable load (s8) from @D0)
+    MOV8mr %stack.0, 1, $noreg, 0, $noreg, killed renamable $al :: (store (s8) into %stack.0)
+    ; reload
+    $al = MOV8rm %stack.0, 1, $noreg, 0, $noreg :: (load (s8) from %stack.0)
+    MOV8mr $rip, 1, $noreg, @U0, $noreg, killed renamable $al :: (store (s8) into @U0)
+    RET 0
+...