[X86] Lower calls with clang.arc.attachedcall bundle

[fhahn]

Summary

This patch adds support for lowering function calls with the
clang.arc.attachedcall bundle. The goal is to expand such calls to the
following sequence of instructions:

callq   @fn
movq  %rax, %rdi
callq   _objc_retainAutoreleasedReturnValue / _objc_unsafeClaimAutoreleasedReturnValue

This sequence of instructions triggers Objective-C runtime optimizations,
hence we want to ensure no instructions get moved in between them.
This patch achieves that by adding a new CALL_RVMARKER ISD node,
which gets turned into the CALL64_RVMARKER pseudo, which eventually gets
expanded into the sequence mentioned above.

The ObjC runtime function to call is determined by the
argument in the bundle, which is passed through as a
target constant to the pseudo.

@ahatanak is working on using this attribute in the front- & middle-end.

Together with the front- & middle-end changes, this should address
PR31925 for X86.

This is the X86 version of 46bc40e50246c1902a1ca7916c8286cb837643ee,
which added similar support for AArch64.

[fhahn]

I am not too familiar with how calls are handled in the X86 backend. I would really appreciate any pointers to cases that this approach might miss.

This is only relevant for Objective-C code on X86, so it would probably be OK to drop support for the CALL32* variants?

[fhahn]

After checking with @ahatanak, we need a slightly different approach on X86 compared to ARM64. I'll update the patch shortly.

[fhahn]

Updated to work with operand bundles as available upstream, emit marker ( movq %rdi, %rax) followed by call to either _objc_retainAutoreleasedReturnValue or _objc_unsafeClaimAutoreleasedReturnValue.

[fhahn]

Fix marker, remove 32bit support, as we will only enable it for x86-64 in the frontend initially.

[fhahn]

Fix issue with pcrel32 calls, properly update RAX reg state (if it is implicit-def dead on the original call, we need to change it to implicit-def, because we add a new use; also set the state on the generated call); add reg mask for new call.

[fhahn]

Update bundle to clang.arc.attachedcall, in line with committed bundle name.

[fhahn]

Rebase & ping :)

I also added a new test case to make sure implicit operands for call arguments are added correctly to the generated calls.

[ab]

Can you (or @ahatanak) explain why/how this needs to be different from aarch64? In particular I don't think I understand why you need to emit the retainARV/claimARV calls this late, that seems like quite a drastic difference in behavior for the bundle.
I read through the discussion in D92808 but it sounds like that predates the x86_64-specific behavior, right? But skimming around the various passes involved I'm not actually sure who emits the retainARV/claimARV calls on aarch64.

llvm/lib/Target/X86/X86ISelLowering.cpp

4400

clang.arc.rv -> clang.arc.attachedcall, right? same below

4408

Sel is a bit confusing, especially in an objc context ;)

[fhahn]

Thanks for taking a look!

In D94597#2632806, @ab wrote:

Can you (or @ahatanak) explain why/how this needs to be different from aarch64? In particular I don't think I understand why you need to emit the retainARV/claimARV calls this late, that seems like quite a drastic difference in behavior for the bundle.

Unfortunately the ObjC runtime matches different sequences on AArc64 and X86. This is documented here https://github.com/opensource-apple/objc4/blob/master/runtime/objc-object.h#L974

On AArch64, the callee only checks for mov x29, x29 in the caller and the position of the claim/autorelease runtime call does not matter, which makes things much simpler. On X86_64, it specifically looks for the marker instructions, immediately followed by the runtime call. So we want to make sure that it is very hard to break up this specific sequence, to avoid missing out on the runtime optimization.

I read through the discussion in D92808 but it sounds like that predates the x86_64-specific behavior, right? But skimming around the various passes involved I'm not actually sure who emits the retainARV/claimARV calls on aarch64.

On AArch64 I think that happens during the lowering of the ARC intrinsics pre-ISel, but @ahatanak would know more about the exact place.

[fhahn]

Update references to arc bundle name, change Sel to RuntimeCallType.

[ahatanak]

In D94597#2632887, @fhahn wrote:

In D94597#2632806, @ab wrote:

I read through the discussion in D92808 but it sounds like that predates the x86_64-specific behavior, right? But skimming around the various passes involved I'm not actually sure who emits the retainARV/claimARV calls on aarch64.

On AArch64 I think that happens during the lowering of the ARC intrinsics pre-ISel, but @ahatanak would know more about the exact place.

ARC contract pass inserts the retainRV/claimRV calls before transforming the function. After the transformation is done, the pass removes the inserted runtime calls on x86-64, but leaves the calls in the IR on AArch64.

On AArch64, objc_autoreleaseReturnValue, which the callee calls before returning, looks for only the marker instruction, so it's not necessary for the retainRV/claimRV instructions to immediately follow the function call and the marker instruction. On x86-64, it looks for a movq first and then a retainRV/claimRV call immediately following the movq.

See callerAcceptsOptimizedReturn in the code below:

https://opensource.apple.com/source/objc4/objc4-818.2/runtime/objc-object.h.auto.html

[fhahn]

ping :)

[ab]

In D94597#2632887, @fhahn wrote:

Unfortunately the ObjC runtime matches different sequences on AArc64 and X86. This is documented here https://github.com/opensource-apple/objc4/blob/master/runtime/objc-object.h#L974

On AArch64, the callee only checks for mov x29, x29 in the caller and the position of the claim/autorelease runtime call does not matter, which makes things much simpler. On X86_64, it specifically looks for the marker instructions, immediately followed by the runtime call. So we want to make sure that it is very hard to break up this specific sequence, to avoid missing out on the runtime optimization.

Interesting, I didn't know that! I guess the mov rax/rdi was a natural part of the sequence on x86_64, and the old original runtime check was opportunistically based on the existing codegen.

So, looking at the patch, this looks correct, but I'm still mildly uncomfortable with the sel bit surviving that late. Here's a maybe-terrible idea: would it work if you replaced the i64 0/1 in the bundle with the actual pointer to the called function? e.g.,

%r = call i8* @foo(i64 %c, i64 %b, i64 %a) [ "clang.arc.attachedcall"(i8*(i8*)* @_objc_retainAutoreleasedReturnValue) ]

on aarch64 you just do the bundle with no args (though "attachedcall" may not be the best name at that point); on x86 you make the frontend pass the actual function it wants (edit: though it sounds like the ARC passes would need more of that logic than the frontend?), and the backend doesn't care and just turns the GV into a target globaladdr, and turns it into a call later

[fhahn]

In D94597#2720350, @ab wrote:

In D94597#2632887, @fhahn wrote:

Unfortunately the ObjC runtime matches different sequences on AArc64 and X86. This is documented here https://github.com/opensource-apple/objc4/blob/master/runtime/objc-object.h#L974

On AArch64, the callee only checks for mov x29, x29 in the caller and the position of the claim/autorelease runtime call does not matter, which makes things much simpler. On X86_64, it specifically looks for the marker instructions, immediately followed by the runtime call. So we want to make sure that it is very hard to break up this specific sequence, to avoid missing out on the runtime optimization.

Interesting, I didn't know that! I guess the mov rax/rdi was a natural part of the sequence on x86_64, and the old original runtime check was opportunistically based on the existing codegen.

So, looking at the patch, this looks correct, but I'm still mildly uncomfortable with the sel bit surviving that late. Here's a maybe-terrible idea: would it work if you replaced the i64 0/1 in the bundle with the actual pointer to the called function? e.g.,

%r = call i8* @foo(i64 %c, i64 %b, i64 %a) [ "clang.arc.attachedcall"(i8*(i8*)* @_objc_retainAutoreleasedReturnValue) ]

on aarch64 you just do the bundle with no args (though "attachedcall" may not be the best name at that point); on x86 you make the frontend pass the actual function it wants (edit: though it sounds like the ARC passes would need more of that logic than the frontend?), and the backend doesn't care and just turns the GV into a target globaladdr, and turns it into a call later

Yeah I think that would be a good option. @ahatanak WDYT?

[ahatanak]

Yes, I think that's better. The verifier currently rejects that IR since taking addresses of intrinsic functions isn't allowed (see https://reviews.llvm.org/D92808#2558928), but probably that can be fixed.

[ab]

Ah sorry, I didn't see you tried that already! I don't think we want to weaken the verifier check, this is probably the only case where it makes sense to take the address. I guess we've meandered through the same decision process, thanks for the patience ;) But it might be worth explaining these decisions in the commit message. Also in the langref paragraph for the bundle, it says the call is always emitted for the bundle.
Which brings another idea: how about emitting the call when lowering the bundle on AArch64 as well? There's not much room for better regalloc or other optimizations in the sequence anyway, right?

But back to x86: there's a couple things we could do to make the behavior more obvious (split the bundle into distinct retain/claim ones? replace the sel imm MO in these pseudos with an actual GV/ES reference to the symbol?). But I'm not sure that makes a real difference, so LGTM as-is

llvm/lib/Target/X86/X86ExpandPseudo.cpp

232

why "Cache"?

llvm/test/CodeGen/X86/call-rv-marker.ll

142

Hmm, I'm curious, do you know why this doesn't trigger? Is it just missing entries in the fold tables? (does the extra operand make that trickier than it should be?)

[fhahn]

Thanks Ahmed! I'll address your comment and then I'll land it. I expect that we will probably be making a couple of improvements once this lands and gets wider test coverage

llvm/lib/Target/X86/X86ExpandPseudo.cpp

232

I'm not sure any more, I'll rename it to Fn :)

llvm/test/CodeGen/X86/call-rv-marker.ll

142

Yes I think there's an entry missing to optimize this. I'll investigate that separately.

llvm/lib/Target/X86/X86ExpandPseudo.cpp

public:
 
   StringRef getPassName() const override {
     return "X86 pseudo instruction expansion pass";
   }
 
 private:
   void ExpandICallBranchFunnel(MachineBasicBlock *MBB,
                                MachineBasicBlock::iterator MBBI);
-
+  void expandCALL_RVMARKER(MachineBasicBlock &MBB,
+                           MachineBasicBlock::iterator MBBI);
   bool ExpandMI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI);
   bool ExpandMBB(MachineBasicBlock &MBB);
 };
 char X86ExpandPseudo::ID = 0;
 
 } // End anonymous namespace.
 
 INITIALIZE_PASS(X86ExpandPseudo, DEBUG_TYPE, X86_EXPAND_PSEUDO_NAME, false,
void X86ExpandPseudo::ExpandICallBranchFunnel(
   for (auto P : TargetMBBs) {
     MF->insert(InsPt, P.first);
     BuildMI(P.first, DL, TII->get(X86::TAILJMPd64))
         .add(JTInst->getOperand(3 + 2 * P.second));
   }
   JTMBB->erase(JTInst);
 }
 
+void X86ExpandPseudo::expandCALL_RVMARKER(MachineBasicBlock &MBB,
+                                          MachineBasicBlock::iterator MBBI) {
+  // Expand CALL_RVMARKER pseudo to call instruction, followed by the special
+  //"movl %ebp, %ebp" marker.
+  // TODO: Mark the sequence as bundle, to avoid passes moving other code
+  // in between.
+  MachineInstr &MI = *MBBI;
+
+  MachineInstr *OriginalCall;
+  MachineOperand &CallTarget = MI.getOperand(0);
+  assert((CallTarget.isGlobal() || CallTarget.isReg()) &&
+         "invalid operand for regular call");
+  unsigned Opc =
+      MI.getOpcode() == X86::CALL32_RVMARKER ? X86::CALL32r : X86::CALL64r;
+  if (MI.getOpcode() == X86::CALL64m_RVMARKER)
+    Opc = X86::CALL64m;
+  else if (CallTarget.isGlobal() || CallTarget.isSymbol())
+    Opc = MI.getOpcode() == X86::CALL32_RVMARKER ? X86::CALLpcrel32
+                                                 : X86::CALL64pcrel32;
+  OriginalCall = BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(Opc)).getInstr();
+  OriginalCall->addOperand(CallTarget);
+
+  unsigned RegMaskStartIdx = 1;
+  // Skip register arguments. Those are added during ISel, but are not
+  // needed for the concrete branch.
+  while (!MI.getOperand(RegMaskStartIdx).isRegMask()) {
+    assert(MI.getOperand(RegMaskStartIdx).isReg() &&
+           "should only skip register operands");
+    RegMaskStartIdx++;
+  }
+  for (; RegMaskStartIdx < MI.getNumOperands(); ++RegMaskStartIdx)
+    OriginalCall->addOperand(MI.getOperand(RegMaskStartIdx));
+
+  // Emit marker "movl %ebp, %ebp".
+  auto *Marker = BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(X86::MOV32rr))
+                     .addReg(X86::EBP, RegState::Define)
+                     .addReg(X86::EBP)
+                     .getInstr();
+  if (MI.shouldUpdateCallSiteInfo())
+    MBB.getParent()->moveCallSiteInfo(&MI, Marker);
+  MI.eraseFromParent();
+}
+
 /// If \p MBBI is a pseudo instruction, this method expands
 /// it to the corresponding (sequence of) actual instruction(s).
 /// \returns true if \p MBBI has been expanded.
 bool X86ExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
                                MachineBasicBlock::iterator MBBI) {
   MachineInstr &MI = *MBBI;
   unsigned Opcode = MI.getOpcode();
   DebugLoc DL = MBBI->getDebugLoc();
   switch (Opcode) {
   default:
     return false;
   case X86::TCRETURNdi:
   case X86::TCRETURNdicc:
   case X86::TCRETURNri:
   case X86::TCRETURNmi:
   case X86::TCRETURNdi64:
   case X86::TCRETURNdi64cc:
   case X86::TCRETURNri64:
   case X86::TCRETURNmi64: {
     bool isMem = Opcode == X86::TCRETURNmi || Opcode == X86::TCRETURNmi64;
     MachineOperand &JumpTarget = MBBI->getOperand(0);
case X86::PTILESTOREDV: {
     return true;
   }
   case X86::PTILEZEROV: {
     for (int i = 3; i > 0; --i) // Remove row, col, $tmmcfg
       MI.RemoveOperand(i);
     MI.setDesc(TII->get(X86::TILEZERO));
     return true;
   }
+  case X86::CALL32_RVMARKER:
+  case X86::CALL64_RVMARKER:
+  case X86::CALL64m_RVMARKER:
+    expandCALL_RVMARKER(MBB, MBBI);
+    return true;
   }
   llvm_unreachable("Previous switch has a fallthrough?");
 }
 
 /// Expand all pseudo instructions contained in \p MBB.
 /// \returns true if any expansion occurred for \p MBB.
 bool X86ExpandPseudo::ExpandMBB(MachineBasicBlock &MBB) {
   bool Modified = false;

llvm/lib/Target/X86/X86ISelLowering.h

enum NodeType : unsigned {
     ///     #1 - The first register result value (optional)
     ///     #2 - The second register result value (optional)
     ///
     CALL,
 
     /// Same as call except it adds the NoTrack prefix.
     NT_CALL,
 
+    // Pseudo for a OBJC call that gets emitted together with a special
+    // marker instruction.
+    CALL_RVMARKER,
+
     /// X86 compare and logical compare instructions.
     CMP,
     FCMP,
     COMI,
     UCOMI,
 
     /// X86 bit-test instructions.
     BT,

llvm/lib/Target/X86/X86ISelLowering.cpp

if (isTailCall) {
     SDValue Ret = DAG.getNode(X86ISD::TC_RETURN, dl, NodeTys, Ops);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
   if (HasNoCfCheck && IsCFProtectionSupported && IsIndirectCall) {
     Chain = DAG.getNode(X86ISD::NT_CALL, dl, NodeTys, Ops);
   } else {
     Chain = DAG.getNode(X86ISD::CALL, dl, NodeTys, Ops);
   }
+
+  // Calls marked with "rv_marker" are special. They should be expanded to the
+  // call, directly followed by a special marker sequence. Use the CALL_RVMARKER
+  // to do that.
+  if (CLI.CB && CLI.CB->hasRetAttr("rv_marker")) {
+    assert(!isTailCall && "tail calls cannot be marked with rv_marker");
+    Chain = DAG.getNode(X86ISD::CALL_RVMARKER, dl, NodeTys, Ops);
+  }
+
   InFlag = Chain.getValue(1);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   DAG.addCallSiteInfo(Chain.getNode(), std::move(CSInfo));
 
   // Save heapallocsite metadata.
   if (CLI.CB)
     if (MDNode *HeapAlloc = CLI.CB->getMetadata("heapallocsite"))
       DAG.addHeapAllocSite(Chain.getNode(), HeapAlloc);
#define NODE_NAME_CASE(NODE) case X86ISD::NODE: return "X86ISD::" #NODE;
   NODE_NAME_CASE(FOR)
   NODE_NAME_CASE(FXOR)
   NODE_NAME_CASE(FILD)
   NODE_NAME_CASE(FIST)
   NODE_NAME_CASE(FP_TO_INT_IN_MEM)
   NODE_NAME_CASE(FLD)
   NODE_NAME_CASE(FST)
   NODE_NAME_CASE(CALL)
+  NODE_NAME_CASE(CALL_RVMARKER)
   NODE_NAME_CASE(BT)
   NODE_NAME_CASE(CMP)
   NODE_NAME_CASE(FCMP)
   NODE_NAME_CASE(STRICT_FCMP)
   NODE_NAME_CASE(STRICT_FCMPS)
   NODE_NAME_CASE(COMI)
   NODE_NAME_CASE(UCOMI)
   NODE_NAME_CASE(CMPM)

llvm/lib/Target/X86/X86InstrControl.td

let isPseudo = 1, isCall = 1, isCodeGenOnly = 1,
   let isTerminator = 1, isReturn = 1, isBarrier = 1 in {
     def INDIRECT_THUNK_TCRETURN64 :
       PseudoI<(outs), (ins GR64:$dst, i32imm:$offset), []>;
     def INDIRECT_THUNK_TCRETURN32 :
       PseudoI<(outs), (ins GR32:$dst, i32imm:$offset), []>;
   }
 }
 
+let isPseudo = 1, isCall = 1, isCodeGenOnly = 1,
+    Uses = [RSP, SSP],
+    SchedRW = [WriteJump] in {
+  def CALL64m_RVMARKER :
+     PseudoI<(outs), (ins i64mem:$dst), [(X86call_rvmarker (loadi64 addr:$dst))]>,
+             Requires<[In64BitMode]>;
+
+  def CALL32_RVMARKER :
+    PseudoI<(outs), (ins GR32:$dst), [(X86call_rvmarker GR32:$dst)]>,
+            Requires<[Not64BitMode]>;
+
+  def CALL64_RVMARKER :
+    PseudoI<(outs), (ins GR64:$dst), [(X86call_rvmarker GR64:$dst)]>,
+            Requires<[In64BitMode]>;
+}
+
 // Conditional tail calls are similar to the above, but they are branches
 // rather than barriers, and they use EFLAGS.
 let isCall = 1, isTerminator = 1, isReturn = 1, isBranch = 1,
     isCodeGenOnly = 1, SchedRW = [WriteJump] in
   let Uses = [RSP, EFLAGS, SSP] in {
   def TCRETURNdi64cc : PseudoI<(outs),
                            (ins i64i32imm_brtarget:$dst, i32imm:$offset,
                             i32imm:$cond), []>;
 
   // This gets substituted to a conditional jump instruction in MC lowering.
   def TAILJMPd64_CC : PseudoI<(outs),
                               (ins i64i32imm_brtarget:$dst, i32imm:$cond), []>;
 }

llvm/lib/Target/X86/X86InstrInfo.td

 def X86callseq_end :
                  SDNode<"ISD::CALLSEQ_END",   SDT_X86CallSeqEnd,
                         [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue]>;
 
 def X86call    : SDNode<"X86ISD::CALL",     SDT_X86Call,
                         [SDNPHasChain, SDNPOutGlue, SDNPOptInGlue,
                          SDNPVariadic]>;
 
+def X86call_rvmarker  : SDNode<"X86ISD::CALL_RVMARKER",     SDT_X86Call,
+                        [SDNPHasChain, SDNPOutGlue, SDNPOptInGlue,
+                         SDNPVariadic]>;
+
+
 def X86NoTrackCall : SDNode<"X86ISD::NT_CALL", SDT_X86Call,
                             [SDNPHasChain, SDNPOutGlue, SDNPOptInGlue,
                              SDNPVariadic]>;
 def X86NoTrackBrind : SDNode<"X86ISD::NT_BRIND", SDT_X86NtBrind,
                              [SDNPHasChain]>;
 
 def X86rep_stos: SDNode<"X86ISD::REP_STOS", SDTX86RepStr,
                         [SDNPHasChain, SDNPInGlue, SDNPOutGlue, SDNPMayStore]>;

llvm/test/CodeGen/X86/call-rv-marker.ll

 @g = global i8* null, align 8
 @fptr = global i8* ()* null, align 8
 
 define i8* @rv_marker_1() {
 ; CHECK-LABEL:  rv_marker_1:
 ; CHECK:         pushq %rax
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    callq   _foo1
+; CHECK-NEXT:    movl    %ebp, %ebp
 ; CHECK-NEXT:    popq    %rcx
 ; CHECK-NEXT:    retq
 ;
 entry:
   %call = call "rv_marker" i8* @foo1()
   ret i8* %call
 }
 
 define void @rv_marker_2_select(i32 %c) {
 ; CHECK-LABEL: rv_marker_2_select:
 ; CHECK:        pushq   %rax
 ; CHECK-NEXT:   .cfi_def_cfa_offset 16
 ; CHECK-NEXT:   cmpl    $1, %edi
 ; CHECK-NEXT:   movl    $1, %edi
 ; CHECK-NEXT:   adcl    $0, %edi
 ; CHECK-NEXT:   callq   _foo0
+; CHECK-NEXT:   movl    %ebp, %ebp
 ; CHECK-NEXT:   movq    %rax, %rdi
 ; CHECK-NEXT:   popq    %rax
 ; CHECK-NEXT:   jmp _foo2
 ;
 entry:
   %tobool.not = icmp eq i32 %c, 0
   %.sink = select i1 %tobool.not, i32 2, i32 1
   %call1 = call "rv_marker" i8* @foo0(i32 %.sink)
   tail call void @foo2(i8* %call1)
   ret void
 }
 
 define void @rv_marker_3() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; CHECK-LABEL: rv_marker_3
 ; CHECK:         pushq   %r14
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    pushq   %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 24
 ; CHECK-NEXT:    pushq   %rax
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset %rbx, -24
 ; CHECK-NEXT:    .cfi_offset %r14, -16
 ; CHECK-NEXT:    callq   _foo1
+; CHECK-NEXT:    movl    %ebp, %ebp
 ; CHECK-NEXT:    movq    %rax, %rbx
 ; CHECK-NEXT: Ltmp0:
 ;
 entry:
   %call = call "rv_marker" i8* @foo1()
   invoke void @objc_object(i8* %call) #5
           to label %invoke.cont unwind label %lpad
 

 ; CHECK-NEXT:    pushq   %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 24
 ; CHECK-NEXT:    pushq   %rax
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset %rbx, -24
 ; CHECK-NEXT:    .cfi_offset %r14, -16
 ; CHECK-NEXT: Ltmp3:
 ; CHECK-NEXT:    callq   _foo1
+; CHECK-NEXT:    movl    %ebp, %ebp
 ; CHECK-NEXT: Ltmp4:
 ;
 entry:
   %s = alloca %struct.S, align 1
   %0 = getelementptr inbounds %struct.S, %struct.S* %s, i64 0, i32 0
   call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %0) #2
   %call = invoke "rv_marker" i8* @foo1()
           to label %invoke.cont unwind label %lpad

 
 ehcleanup:                                        ; preds = %lpad1, %lpad
   %.pn = phi { i8*, i32 } [ %2, %lpad1 ], [ %1, %lpad ]
   %call4 = call %struct.S* @_ZN1SD1Ev(%struct.S* nonnull dereferenceable(1) %s)
   call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %0)
   resume { i8*, i32 } %.pn
 }
 
+; TODO: This should use "callq *_fptr(%rip)".
 define i8* @rv_marker_5_indirect_call() {
 ; CHECK-LABEL: rv_marker_5_indirect_call
 ; CHECK:         pushq   %rbx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    .cfi_offset %rbx, -16
-; CHECK-NEXT:    callq   *_fptr(%rip)
+; CHECK-NEXT:    movq    _fptr(%rip), %rax
+; CHECK-NEXT:    callq   *%rax
+; CHECK-NEXT:    movl    %ebp, %ebp
 ; CHECK-NEXT:    movq    %rax, %rbx
 ; CHECK-NEXT:    movq    %rax, %rdi
 ; CHECK-NEXT:    callq   _foo2
 ; CHECK-NEXT:    movq    %rbx, %rax
 ; CHECK-NEXT:    popq    %rbx
 ; CHECK-NEXT:    retq
 ;
 entry:
   %0 = load i8* ()*, i8* ()** @fptr, align 8
   %call = call "rv_marker" i8* %0()
   tail call void @foo2(i8* %call)
   ret i8* %call
 }
 
 declare void @foo(i64, i64, i64)
 
 define void @rv_marker_multiarg(i64 %a, i64 %b, i64 %c) {
 ; CHECK-LABEL: rv_marker_multiarg
 ; CHECK:         pushq   %rax
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    movq    %rdi, %rax
 ; CHECK-NEXT:    movq    %rdx, %rdi
 ; CHECK-NEXT:    movq    %rax, %rdx
 ; CHECK-NEXT:    callq   _foo
+; CHECK-NEXT:    movl    %ebp, %ebp
 ; CHECK-NEXT:    popq    %rax
 ; CHECK-NEXT:    retq
 ;
   call "rv_marker" void @foo(i64 %c, i64 %b, i64 %a)
   ret void
 }
 
 declare i32 @__gxx_personality_v0(...)