[X86] Block UndefRegUpdate

[tkrupa]

Summary

Prevent folding of operations with memory loads when one of the sources has undefined register update.

[craig.topper]

I'm going to accept this because AVX and SSE are consistent on cvtsi2ss and similar instructions now. But we should file a bug to fix both.

test/CodeGen/X86/fast-isel-int-float-conversion-x86-64.ll

80 ↗

(On Diff #149407)

This did nothing to prevent an undef register update. The load goes to a GPR so it didn't do any good. But it is consistent with what was done for the SSE intruction.

llvm/trunk/lib/Target/X86/X86InstrInfo.cpp

if (OpNum == 2) {
       }
     }
     break;
   };
 
   return nullptr;
 }
 
+static bool shouldPreventUndefRegUpdateMemFold(MachineFunction &MF, MachineInstr &MI) {
+  if (MF.getFunction().optForSize() || !hasUndefRegUpdate(MI.getOpcode()) ||
+      !MI.getOperand(1).isReg())
+    return false;
+ 
+  MachineRegisterInfo &RegInfo = MF.getRegInfo();
+  MachineInstr *VRegDef = RegInfo.getUniqueVRegDef(MI.getOperand(1).getReg());
+  if (VRegDef == nullptr)
+    return false;
+  return VRegDef->isImplicitDef();
+}
+
+
 MachineInstr *X86InstrInfo::foldMemoryOperandImpl(
     MachineFunction &MF, MachineInstr &MI, unsigned OpNum,
     ArrayRef<MachineOperand> MOs, MachineBasicBlock::iterator InsertPt,
     unsigned Size, unsigned Align, bool AllowCommute) const {
   const DenseMap<unsigned,
                  std::pair<uint16_t, uint16_t> > *OpcodeTablePtr = nullptr;
   bool isSlowTwoMemOps = Subtarget.slowTwoMemOps();
   bool isTwoAddrFold = false;
 
   // For CPUs that favor the register form of a call or push,
   // do not fold loads into calls or pushes, unless optimizing for size
   // aggressively.
   if (isSlowTwoMemOps && !MF.getFunction().optForMinSize() &&
       (MI.getOpcode() == X86::CALL32r || MI.getOpcode() == X86::CALL64r ||
        MI.getOpcode() == X86::PUSH16r || MI.getOpcode() == X86::PUSH32r ||
        MI.getOpcode() == X86::PUSH64r))
     return nullptr;
 
-  // Avoid partial register update stalls unless optimizing for size.
-  // TODO: we should block undef reg update as well.
+  // Avoid partial and undef register update stalls unless optimizing for size.
   if (!MF.getFunction().optForSize() &&
-      hasPartialRegUpdate(MI.getOpcode(), Subtarget))
+      (hasPartialRegUpdate(MI.getOpcode(), Subtarget) ||
+       shouldPreventUndefRegUpdateMemFold(MF, MI)))
     return nullptr;
 
   unsigned NumOps = MI.getDesc().getNumOperands();
   bool isTwoAddr =
       NumOps > 1 && MI.getDesc().getOperandConstraint(1, MCOI::TIED_TO) != -1;
 
   // FIXME: AsmPrinter doesn't know how to handle
   // X86II::MO_GOT_ABSOLUTE_ADDRESS after folding.

 X86InstrInfo::foldMemoryOperandImpl(MachineFunction &MF, MachineInstr &MI,
                                     ArrayRef<unsigned> Ops,
                                     MachineBasicBlock::iterator InsertPt,
                                     int FrameIndex, LiveIntervals *LIS) const {
   // Check switch flag
   if (NoFusing)
     return nullptr;
 
-  // Unless optimizing for size, don't fold to avoid partial
-  // register update stalls
-  // TODO: we should block undef reg update as well.
+  // Avoid partial and undef register update stalls unless optimizing for size.
   if (!MF.getFunction().optForSize() &&
-      hasPartialRegUpdate(MI.getOpcode(), Subtarget))
+      (hasPartialRegUpdate(MI.getOpcode(), Subtarget) ||
+       shouldPreventUndefRegUpdateMemFold(MF, MI)))
     return nullptr;
 
   // Don't fold subreg spills, or reloads that use a high subreg.
   for (auto Op : Ops) {
     MachineOperand &MO = MI.getOperand(Op);
     auto SubReg = MO.getSubReg();
     if (SubReg && (MO.isDef() || SubReg == X86::sub_8bit_hi))
       return nullptr;
if (isLoadFromStackSlot(LoadMI, FrameIndex)) {
     if (isNonFoldablePartialRegisterLoad(LoadMI, MI, MF))
       return nullptr;
     return foldMemoryOperandImpl(MF, MI, Ops, InsertPt, FrameIndex, LIS);
   }
 
   // Check switch flag
   if (NoFusing) return nullptr;
 
-  // Avoid partial register update stalls unless optimizing for size.
-  // TODO: we should block undef reg update as well.
+  // Avoid partial and undef register update stalls unless optimizing for size.
   if (!MF.getFunction().optForSize() &&
-      hasPartialRegUpdate(MI.getOpcode(), Subtarget))
+      (hasPartialRegUpdate(MI.getOpcode(), Subtarget) ||
+       shouldPreventUndefRegUpdateMemFold(MF, MI)))
     return nullptr;
 
   // Determine the alignment of the load.
   unsigned Alignment = 0;
   if (LoadMI.hasOneMemOperand())
     Alignment = (*LoadMI.memoperands_begin())->getAlignment();
   else
     switch (LoadMI.getOpcode()) {

llvm/trunk/test/CodeGen/X86/fast-isel-int-float-conversion-x86-64.ll

 ; SSE2-LABEL: long_to_double_rm:
 ; SSE2:       # %bb.0: # %entry
 ; SSE2-NEXT:    movq (%rdi), %rax
 ; SSE2-NEXT:    cvtsi2sdq %rax, %xmm0
 ; SSE2-NEXT:    retq
 ;
 ; AVX-LABEL: long_to_double_rm:
 ; AVX:       # %bb.0: # %entry
-; AVX-NEXT:    vcvtsi2sdq (%rdi), %xmm0, %xmm0
+; AVX-NEXT:    movq (%rdi), %rax
+; AVX-NEXT:    vcvtsi2sdq %rax, %xmm0, %xmm0
 ; AVX-NEXT:    retq
 entry:
   %0 = load i64, i64* %a
   %1 = sitofp i64 %0 to double
   ret double %1
 }
 
 define double @long_to_double_rm_optsize(i64* %a) optsize {

 ; SSE2-LABEL: long_to_float_rm:
 ; SSE2:       # %bb.0: # %entry
 ; SSE2-NEXT:    movq (%rdi), %rax
 ; SSE2-NEXT:    cvtsi2ssq %rax, %xmm0
 ; SSE2-NEXT:    retq
 ;
 ; AVX-LABEL: long_to_float_rm:
 ; AVX:       # %bb.0: # %entry
-; AVX-NEXT:    vcvtsi2ssq (%rdi), %xmm0, %xmm0
+; AVX-NEXT:    movq (%rdi), %rax
+; AVX-NEXT:    vcvtsi2ssq %rax, %xmm0, %xmm0
 ; AVX-NEXT:    retq
 entry:
   %0 = load i64, i64* %a
   %1 = sitofp i64 %0 to float
   ret float %1
 }
 
 define float @long_to_float_rm_optsize(i64* %a) optsize {

llvm/trunk/test/CodeGen/X86/fast-isel-int-float-conversion.ll

 ; AVX_X86:       # %bb.0: # %entry
 ; AVX_X86-NEXT:    pushl %ebp
 ; AVX_X86-NEXT:    .cfi_def_cfa_offset 8
 ; AVX_X86-NEXT:    .cfi_offset %ebp, -8
 ; AVX_X86-NEXT:    movl %esp, %ebp
 ; AVX_X86-NEXT:    .cfi_def_cfa_register %ebp
 ; AVX_X86-NEXT:    andl $-8, %esp
 ; AVX_X86-NEXT:    subl $8, %esp
-; AVX_X86-NEXT:    vcvtsi2sdl 8(%ebp), %xmm0, %xmm0
+; AVX_X86-NEXT:    movl 8(%ebp), %eax
+; AVX_X86-NEXT:    vcvtsi2sdl %eax, %xmm0, %xmm0
 ; AVX_X86-NEXT:    vmovsd %xmm0, (%esp)
 ; AVX_X86-NEXT:    fldl (%esp)
 ; AVX_X86-NEXT:    movl %ebp, %esp
 ; AVX_X86-NEXT:    popl %ebp
 ; AVX_X86-NEXT:    .cfi_def_cfa %esp, 4
 ; AVX_X86-NEXT:    retl
 entry:
   %0 = sitofp i32 %a to double
   ret double %0
 }
 
 define double @int_to_double_rm(i32* %a) {
 ; SSE2-LABEL: int_to_double_rm:
 ; SSE2:       # %bb.0: # %entry
 ; SSE2-NEXT:    movl (%rdi), %eax
 ; SSE2-NEXT:    cvtsi2sdl %eax, %xmm0
 ; SSE2-NEXT:    retq
 ;
 ; AVX-LABEL: int_to_double_rm:
 ; AVX:       # %bb.0: # %entry
-; AVX-NEXT:    vcvtsi2sdl (%rdi), %xmm0, %xmm0
+; AVX-NEXT:    movl (%rdi), %eax
+; AVX-NEXT:    vcvtsi2sdl %eax, %xmm0, %xmm0
 ; AVX-NEXT:    retq
 ;
 ; SSE2_X86-LABEL: int_to_double_rm:
 ; SSE2_X86:       # %bb.0: # %entry
 ; SSE2_X86-NEXT:    pushl %ebp
 ; SSE2_X86-NEXT:    .cfi_def_cfa_offset 8
 ; SSE2_X86-NEXT:    .cfi_offset %ebp, -8
 ; SSE2_X86-NEXT:    movl %esp, %ebp

 ; SSE2_X86-NEXT:    popl %eax
 ; SSE2_X86-NEXT:    .cfi_def_cfa_offset 4
 ; SSE2_X86-NEXT:    retl
 ;
 ; AVX_X86-LABEL: int_to_float_rr:
 ; AVX_X86:       # %bb.0: # %entry
 ; AVX_X86-NEXT:    pushl %eax
 ; AVX_X86-NEXT:    .cfi_def_cfa_offset 8
-; AVX_X86-NEXT:    vcvtsi2ssl {{[0-9]+}}(%esp), %xmm0, %xmm0
+; AVX_X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; AVX_X86-NEXT:    vcvtsi2ssl %eax, %xmm0, %xmm0
 ; AVX_X86-NEXT:    vmovss %xmm0, (%esp)
 ; AVX_X86-NEXT:    flds (%esp)
 ; AVX_X86-NEXT:    popl %eax
 ; AVX_X86-NEXT:    .cfi_def_cfa_offset 4
 ; AVX_X86-NEXT:    retl
 entry:
   %0 = sitofp i32 %a to float
   ret float %0
 }
 
 define float @int_to_float_rm(i32* %a) {
 ; SSE2-LABEL: int_to_float_rm:
 ; SSE2:       # %bb.0: # %entry
 ; SSE2-NEXT:    movl (%rdi), %eax
 ; SSE2-NEXT:    cvtsi2ssl %eax, %xmm0
 ; SSE2-NEXT:    retq
 ;
 ; AVX-LABEL: int_to_float_rm:
 ; AVX:       # %bb.0: # %entry
-; AVX-NEXT:    vcvtsi2ssl (%rdi), %xmm0, %xmm0
+; AVX-NEXT:    movl (%rdi), %eax
+; AVX-NEXT:    vcvtsi2ssl %eax, %xmm0, %xmm0
 ; AVX-NEXT:    retq
 ;
 ; SSE2_X86-LABEL: int_to_float_rm:
 ; SSE2_X86:       # %bb.0: # %entry
 ; SSE2_X86-NEXT:    pushl %eax
 ; SSE2_X86-NEXT:    .cfi_def_cfa_offset 8
 ; SSE2_X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; SSE2_X86-NEXT:    cvtsi2ssl (%eax), %xmm0

llvm/trunk/test/CodeGen/X86/vector-sqrt.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx  | FileCheck %s
 
 ; Function Attrs: nounwind readonly uwtable
 define <2 x double> @sqrtd2(double* nocapture readonly %v) local_unnamed_addr #0 {
 ; CHECK-LABEL: sqrtd2:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    vsqrtsd (%rdi), %xmm0, %xmm0
-; CHECK-NEXT:    vsqrtsd 8(%rdi), %xmm1, %xmm1
+; CHECK-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
+; CHECK-NEXT:    vmovsd {{.*#+}} xmm1 = mem[0],zero
+; CHECK-NEXT:    vsqrtsd %xmm0, %xmm0, %xmm0
+; CHECK-NEXT:    vsqrtsd %xmm1, %xmm1, %xmm1
 ; CHECK-NEXT:    vunpcklpd {{.*#+}} xmm0 = xmm0[0],xmm1[0]
 ; CHECK-NEXT:    retq
 entry:
   %0 = load double, double* %v, align 8
   %call = tail call double @sqrt(double %0) #2
   %arrayidx1 = getelementptr inbounds double, double* %v, i64 1
   %1 = load double, double* %arrayidx1, align 8
   %call2 = tail call double @sqrt(double %1) #2
   %vecinit.i = insertelement <2 x double> undef, double %call, i32 0
   %vecinit1.i = insertelement <2 x double> %vecinit.i, double %call2, i32 1
   ret <2 x double> %vecinit1.i
 }
 
 ; Function Attrs: nounwind readnone
 declare double @sqrt(double) local_unnamed_addr #1
 
 ; Function Attrs: nounwind readonly uwtable
 define <4 x float> @sqrtf4(float* nocapture readonly %v) local_unnamed_addr #0 {
 ; CHECK-LABEL: sqrtf4:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    vsqrtss (%rdi), %xmm0, %xmm0
-; CHECK-NEXT:    vsqrtss 4(%rdi), %xmm1, %xmm1
-; CHECK-NEXT:    vsqrtss 8(%rdi), %xmm2, %xmm2
-; CHECK-NEXT:    vsqrtss 12(%rdi), %xmm3, %xmm3
+; CHECK-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; CHECK-NEXT:    vmovss {{.*#+}} xmm1 = mem[0],zero,zero,zero
+; CHECK-NEXT:    vsqrtss %xmm0, %xmm0, %xmm0
+; CHECK-NEXT:    vsqrtss %xmm1, %xmm1, %xmm1
+; CHECK-NEXT:    vmovss {{.*#+}} xmm2 = mem[0],zero,zero,zero
+; CHECK-NEXT:    vsqrtss %xmm2, %xmm2, %xmm2
+; CHECK-NEXT:    vmovss {{.*#+}} xmm3 = mem[0],zero,zero,zero
+; CHECK-NEXT:    vsqrtss %xmm3, %xmm3, %xmm3
 ; CHECK-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0],xmm1[0],xmm0[2,3]
 ; CHECK-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0,1],xmm2[0],xmm0[3]
 ; CHECK-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0,1,2],xmm3[0]
 ; CHECK-NEXT:    retq
 entry:
   %0 = load float, float* %v, align 4
   %call = tail call float @sqrtf(float %0) #2
   %arrayidx1 = getelementptr inbounds float, float* %v, i64 1