X86: don't fold swifterror load into anything

[t.p.northover]

Summary

Swifterror loads aren't what they seem because there's an implicit mini-mem2reg that happens during ISel, effectively removing the stack slot from existence. So it shouldn't be folded into any other arithmetic by FastISel either.

[rjmccall]

I don't have direct expertise in this code, but the idea looks correct to me.

It's a shame that this sort of thing is a problem, of course.

[Gerolf]

LGTM!

llvm/lib/Target/X86/X86FastISel.cpp

private:
   /// computed in an SSE register, not on the X87 floating point stack.
   bool isScalarFPTypeInSSEReg(EVT VT) const {
     return (VT == MVT::f64 && Subtarget->hasSSE2()) ||
            (VT == MVT::f32 && Subtarget->hasSSE1()) || VT == MVT::f16;
   }
 
   bool isTypeLegal(Type *Ty, MVT &VT, bool AllowI1 = false);
 
+  bool isPointerSwiftError(const Value *PtrV);
+
   bool IsMemcpySmall(uint64_t Len);
 
   bool TryEmitSmallMemcpy(X86AddressMode DestAM,
                           X86AddressMode SrcAM, uint64_t Len);
 
   bool foldX86XALUIntrinsic(X86::CondCode &CC, const Instruction *I,
                             const Value *Cond);
 
if (AM.IndexReg == 0) {
       AM.IndexReg = GetCallRegForValue(V);
       return AM.IndexReg != 0;
     }
   }
 
   return false;
 }
 
+// Swifterror values can come from either a function parameter with
+// swifterror attribute or an alloca with swifterror attribute.
+bool X86FastISel::isPointerSwiftError(const Value *PtrV) {
+  if (!TLI.supportSwiftError())
+    return false;
+
+  if (const Argument *Arg = dyn_cast<Argument>(PtrV))
+    if (Arg->hasSwiftErrorAttr())
+      return true;;
+
+  if (const AllocaInst *Alloca = dyn_cast<AllocaInst>(PtrV))
+    if (Alloca->isSwiftError())
+      return true;
+
+  return false;
+}
 
 /// X86SelectStore - Select and emit code to implement store instructions.
 bool X86FastISel::X86SelectStore(const Instruction *I) {
   // Atomic stores need special handling.
   const StoreInst *S = cast<StoreInst>(I);
 
   if (S->isAtomic())
     return false;
 
-  const Value *PtrV = I->getOperand(1);
-  if (TLI.supportSwiftError()) {
-    // Swifterror values can come from either a function parameter with
-    // swifterror attribute or an alloca with swifterror attribute.
-    if (const Argument *Arg = dyn_cast<Argument>(PtrV)) {
-      if (Arg->hasSwiftErrorAttr())
-        return false;
-    }
-
-    if (const AllocaInst *Alloca = dyn_cast<AllocaInst>(PtrV)) {
-      if (Alloca->isSwiftError())
-        return false;
-    }
-  }
+  if (isPointerSwiftError(I->getOperand(1)))
 
   const Value *Val = S->getValueOperand();
   const Value *Ptr = S->getPointerOperand();
 
   MVT VT;
   if (!isTypeLegal(Val->getType(), VT, /*AllowI1=*/true))
     return false;
 

 ///
 bool X86FastISel::X86SelectLoad(const Instruction *I) {
   const LoadInst *LI = cast<LoadInst>(I);
 
   // Atomic loads need special handling.
   if (LI->isAtomic())
     return false;
 
-  const Value *SV = I->getOperand(0);
-  if (TLI.supportSwiftError()) {
-    // Swifterror values can come from either a function parameter with
-    // swifterror attribute or an alloca with swifterror attribute.
-    if (const Argument *Arg = dyn_cast<Argument>(SV)) {
-      if (Arg->hasSwiftErrorAttr())
-        return false;
-    }
-
-    if (const AllocaInst *Alloca = dyn_cast<AllocaInst>(SV)) {
-      if (Alloca->isSwiftError())
-        return false;
-    }
-  }
+  if (isPointerSwiftError(I->getOperand(0)))
 
   MVT VT;
   if (!isTypeLegal(LI->getType(), VT, /*AllowI1=*/true))
     return false;
 
   const Value *Ptr = LI->getPointerOperand();
 
   X86AddressMode AM;
unsigned X86FastISel::fastMaterializeFloatZero(const ConstantFP *CF) {
   BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), ResultReg);
   return ResultReg;
 }
 
 
 bool X86FastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo,
                                       const LoadInst *LI) {
   const Value *Ptr = LI->getPointerOperand();
+
+  if (isPointerSwiftError(Ptr))
+    return false;
+
   X86AddressMode AM;
   if (!X86SelectAddress(Ptr, AM))
     return false;
 
   const X86InstrInfo &XII = (const X86InstrInfo &)TII;
 
   unsigned Size = DL.getTypeAllocSize(LI->getType());
 

llvm/test/CodeGen/X86/swifterror.ll

entry:
   call swiftcc void @foo2(ptr swifterror %err)
   store ptr null, ptr %err
   br label %a
 
 a:
   %error = load ptr, ptr %err
   ret ptr %error
 }
+
+define i32 @nofold_swifterror() {
+; CHECK-LABEL: nofold_swifterror:
+; CHECK: cmpq $0, %r12
+; CHECK-APPLE-LABEL: nofold_swifterror:
+; CHECK-APPLE:       ## %bb.0:
+; CHECK-APPLE-NEXT:    pushq %r12
+; CHECK-APPLE-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-APPLE-NEXT:    subq $16, %rsp
+; CHECK-APPLE-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-APPLE-NEXT:    .cfi_offset %r12, -16
+; CHECK-APPLE-NEXT:    callq _foo
+; CHECK-APPLE-NEXT:    testq %r12, %r12
+; CHECK-APPLE-NEXT:    leaq 16(%rsp), %rsp
+; CHECK-APPLE-NEXT:    popq %r12
+; CHECK-APPLE-NEXT:    jne LBB26_2
+; CHECK-APPLE-NEXT:  ## %bb.1: ## %good
+; CHECK-APPLE-NEXT:    xorl %eax, %eax
+; CHECK-APPLE-NEXT:    retq
+; CHECK-APPLE-NEXT:  LBB26_2: ## %bad
+; CHECK-APPLE-NEXT:    movl $42, %eax
+; CHECK-APPLE-NEXT:    retq
+;
+; CHECK-O0-LABEL: nofold_swifterror:
+; CHECK-O0:       ## %bb.0:
+; CHECK-O0-NEXT:    pushq %r12
+; CHECK-O0-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-O0-NEXT:    subq $16, %rsp
+; CHECK-O0-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-O0-NEXT:    .cfi_offset %r12, -16
+; CHECK-O0-NEXT:    ## implicit-def: $rax
+; CHECK-O0-NEXT:  ## %bb.1: ## %next
+; CHECK-O0-NEXT:    movq (%rsp), %r12 ## 8-byte Reload
+; CHECK-O0-NEXT:    callq _foo
+; CHECK-O0-NEXT:    cmpq $0, %r12
+; CHECK-O0-NEXT:    jne LBB26_3
+; CHECK-O0-NEXT:  ## %bb.2: ## %good
+; CHECK-O0-NEXT:    xorl %eax, %eax
+; CHECK-O0-NEXT:    addq $16, %rsp
+; CHECK-O0-NEXT:    popq %r12
+; CHECK-O0-NEXT:    retq
+; CHECK-O0-NEXT:  LBB26_3: ## %bad
+; CHECK-O0-NEXT:    movl $42, %eax
+; CHECK-O0-NEXT:    addq $16, %rsp
+; CHECK-O0-NEXT:    popq %r12
+; CHECK-O0-NEXT:    retq
+;
+; CHECK-i386-LABEL: nofold_swifterror:
+; CHECK-i386:       ## %bb.0:
+; CHECK-i386-NEXT:    subl $12, %esp
+; CHECK-i386-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-i386-NEXT:    leal 8(%esp), %eax
+; CHECK-i386-NEXT:    movl %eax, (%esp)
+; CHECK-i386-NEXT:    calll _foo
+; CHECK-i386-NEXT:    cmpl $0, 8(%esp)
+; CHECK-i386-NEXT:    leal 12(%esp), %esp
+; CHECK-i386-NEXT:    jne LBB26_2
+; CHECK-i386-NEXT:  ## %bb.1: ## %good
+; CHECK-i386-NEXT:    xorl %eax, %eax
+; CHECK-i386-NEXT:    retl
+; CHECK-i386-NEXT:  LBB26_2: ## %bad
+; CHECK-i386-NEXT:    movl $42, %eax
+; CHECK-i386-NEXT:    retl
+  %se = alloca swifterror ptr, align 8
+  br label %next
+
+next:
+  call void @foo(ptr swifterror %se)
+  %err = load ptr, ptr %se, align 8
+  %tst = icmp eq ptr %err, null
+  br i1 %tst, label %good, label %bad
+
+good:
+  ret i32 0
+
+bad:
+  ret i32 42
+}