[InlineAsm] Support call function label in x86 inline asm with PIC

[xiangzhangllvm]

Summary

In Linux PIC model, Global Address (GA) of Global Variable (GV) will be got from loading GOT slot.
Different instructions has different understanding about using Global Address.

For example in X86:

1. We assign a value to GV, we can use "MOV GV, ...", it corresping to "MOV (Global Address) ...".
2. We got related address of GV, we can use "LEA GV, ...", it corresping to "LEA (Global Address) ...".
3. But if we call the label (GV can be a label), we can use "CALL GV", it didn't equal with "CALL (Global Address)".

So, it is obvious that Global Address of Global Variable should be specially handled case by case.
This is done in normal IR/MIR which has "single" purpose use of the Global Address.
But things changed in inline asm which represented by only one IR/MIR but may contains
a lot of instructions with mult-purpose on same or different Global Address.
What is more, llvm didn't distinguish the instructions in inline asm IR/MIR.

TODO: The other targets may also has this problem, we need to fix them too. It is an arch defect for llvm inline asm.

Take a concrete example: test.c: (clang t.c -fasm-blocks -S -fpic -emit-llvm test.c )

extern void sincos();
int GV=3;
int Arr[10] = {1,};
void foo() {
 asm {
   lea r8, Arr
   mov rax, GV
   lea r9, sincos
   call sincos
   ret }
}

it will generate following wrong code:

movq    sincos@GOTPCREL(%rip), %rsi
movq    GV@GOTPCREL(%rip), %rdx
movq    Arr@GOTPCREL(%rip), %rcx
#APP

leaq    (%rcx), %r8
movq    (%rdx), %rax         //  Not like "MOV GV", CALL don't get the context of the Function sincos
leaq    (%rsi), %r9          //  Not like "LEA sincos", LEA can get the address without Dereference
callq   *(%rsi)              //  It should directly call the sincos --> callq   %rsi  --> in normal case (non-large code model) it should be "**call sincos@PLT**"
retq

#NO_APP

[xiangzhangllvm]

The fail test lower-em-sjlj-indirect-setjmp.ll has no relation with this patch. (remove this patch it still fail)

[xiangzhangllvm]

Update for clang-format

[xiangzhangllvm]

Update {NUM:P} in LangRef

[LuoYuanke]

I have a general question. Can we handle it in X86DAGToDAGISel::PreprocessISelDAG()? We can create a new "X86ISD::Wrapper" and replace the corresponding memory operand of the INLINEASM node with the "X86ISD::Wrapper" node. Here is the pseudo example.

Transform the DAG

t0: ch = EntryToken
    t12: i64 = X86ISD::WrapperRIP TargetGlobalAddress:i64<void (...)* @bar> 0 [TF=5]
  t14: i64,ch = load<(load (s64) from got)> t0, t12, undef:i64
t10: ch,glue = inlineasm t0, TargetExternalSymbol:i64'call qword ptr ${0:P}
      ret', MDNode:ch<0x6b0f848>, TargetConstant:i64<13>, TargetConstant:i32<196622>, t14, TargetConstant:i32<12>, Register:i32 $df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, Register:i32 $eflags

to

t0: ch = EntryToken
    t12: i64 = X86ISD::WrapperRIP TargetGlobalAddress:i64<void (...)* @bar> 0 [TF=5]
  t14: i64,ch = load<(load (s64) from got)> t0, t12, undef:i64
  t120: i64 = X86ISD::Wrapper TargetGlobalAddress:i64<void (...)* @bar> 0
t10: ch,glue = inlineasm t0, TargetExternalSymbol:i64'call qword ptr ${0:P}
      ret', MDNode:ch<0x6b0f848>, TargetConstant:i64<13>, TargetConstant:i32<196622>, t120, TargetConstant:i32<12>, Register:i32 $df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, Register:i32 $eflags

llvm/docs/LangRef.rst

5047 ↗

(On Diff #371496)

Is it accurate? It seems only in PIC mode and in Intel dialect, we print the symbol operand.

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

1189

The syntax should be ensured by front-end when emit IR. Right?

1222

Drop brace.

1246

Is it possible that there are multiple memory operands?

1253

This code is X86 target specific. It assume the sibmem address mod of X86. I think we should refine this function in X86 target.

1254

Why do we assume the 2nd operand is index operand? Is it always true?

1255

Why do we assume index register is noreg?

1396

Maybe create a target function to determine if we want to transform operands for call instruction. The function return false by default.

1467

Drop brace.

1506

Drop the brace.

llvm/lib/Target/X86/X86AsmPrinter.cpp

611

Do we have any test case when MI->getOperand(OpNo) is local symbol?

llvm/test/CodeGen/X86/inline-asm-call.ll

6

Is it necessary to mix -code-model=large with i386-unknown?

gcc -mcmodel=large -m32 t2.c -m32
cc1: error: code model ‘large’ not supported in the 32 bit mode

48

Add "nounwind" to avoid CFI instructions.

83

rsi hold the GOTbase. What does $sincos@GOT means? Is it the sincos entry offset to GOT base?

[pengfei]

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

1436

Maybe we can use j here? Changing to J while leaving i looks odd.

1450

ditto.

[xiangzhangllvm]

In D109407#3038069, @LuoYuanke wrote:

I have a general question. Can we handle it in X86DAGToDAGISel::PreprocessISelDAG()? We can create a new "X86ISD::Wrapper" and replace the corresponding memory operand of the

PreprocessISelDAG is my first idea. The reason I not did it there because in IR/SDNode the GV is corresponding to only 1 value/node, we may have different uses of this GV (e.g. call,lea ...) which was differently lowering according to their users (IRs/SDNodes). For InlineASM, it is different, muti-uses may happened in one User (InlineAsm IR/SDNode ), it is hard distinguish them in lowering.
Another reason I do it here is I want to limited the affection of the change. Here is a good place, It has finish IR/SDNode optimizations, and is just emitting the Inline ASM, all the related info is easy to get.

Add Warpper is a way to reslove the 1 "value/node" problem, but it still need Lowering to add "PLT" flags for call GV which I remember it add by checking its user (not clear by wrapper).
Let me take a look. Thanks for the suggestion and all your reviews.

[xiangzhangllvm]

llvm/docs/LangRef.rst

5047 ↗

(On Diff #371496)

I

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/PBQPRAConstraint.h"
 #include "llvm/CodeGen/ScheduleDAGMutation.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/CodeGen.h"
 #include <memory>
 #include <vector>
 
 namespace llvm {
 
 class CallLowering;
+class GlobalValue;
 class InlineAsmLowering;
 class InstrItineraryData;
 struct InstrStage;
 class InstructionSelector;
 class LegalizerInfo;
 class MachineInstr;
 struct MachineSchedPolicy;
 struct MCReadAdvanceEntry;
public:
 
   /// True if the register allocator should use the allocation orders exactly as
   /// written in the tablegen descriptions, false if it should allocate
   /// the specified physical register later if is it callee-saved.
   virtual bool ignoreCSRForAllocationOrder(const MachineFunction &MF,
                                            unsigned PhysReg) const {
     return false;
   }
+
+  virtual unsigned char classifyGlobalFunctionReference(const GlobalValue *GV) const {
+    return 0;
+  }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_TARGETSUBTARGETINFO_H

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

if (Opc == TargetOpcode::STATEPOINT && NumDefs > 0) {
     }
   }
 
   // Run post-isel target hook to adjust this instruction if needed.
   if (II.hasPostISelHook())
     TLI->AdjustInstrPostInstrSelection(*MIB, Node);
 }
 
+static bool getInlineAsmCallOperands(const char *AsmStr, SDNode *Node,
+                                     SmallSet<unsigned, 2> &CallOpNos) {
+  const char *CurPtr = AsmStr;
+  unsigned NumOperands = Node->getNumOperands();
+  while (*CurPtr) {
+    if (*CurPtr++ != '$') // Operands start with '$'
+      continue;
+
+    if (*CurPtr == '$') { // Skip "$$"
+      ++CurPtr;
+      continue;
+    }
+
+    bool HasCurlyBraces = false;
+    if (*CurPtr == '{') { // ${variable}
+      ++CurPtr;           // Consume '{' character.
+      HasCurlyBraces = true;
+    }
+
+    if (!HasCurlyBraces)
+      continue;
+
+    // If we have ${:xxx}, this is not a real operand reference, skip it.
+    if (*CurPtr == ':') {
+      ++CurPtr;
+      const char *StrStart = CurPtr;
+      const char *StrEnd = strchr(StrStart, '}');
+      if (!StrEnd)
+        report_fatal_error("Unterminated ${:xxx} operand in inline asm"
+                           " string: '" +
+                           Twine(AsmStr) + "'");
+
+      CurPtr = StrEnd + 1;
+      continue;
+    }
+
+    // Get the operand number.
+    const char *IDStart = CurPtr;
+    const char *IDEnd = IDStart;
+    while (isDigit(*IDEnd))
+      ++IDEnd;
+
+    unsigned Val;
+    if (StringRef(IDStart, IDEnd - IDStart).getAsInteger(10, Val))
+      report_fatal_error("Bad $ operand number in inline asm string: '" +
+                         Twine(AsmStr) + "'");
+    CurPtr = IDEnd;
+
+    if (Val >= NumOperands - 1)
+      report_fatal_error("Invalid $ operand number in inline asm string: '" +
+                         Twine(AsmStr) + "'");
+
+    if (*CurPtr != ':') {
+      ++CurPtr;
+      continue;
+    }
+
+    // Check for a modifier character.  Some like ${NUM:MODIFIER}.
+    ++CurPtr; // Consume ':' character.
+
+    if (*CurPtr++ != 'P') // Not call operand
+      continue;
+
+    if (*CurPtr != '}')
+      report_fatal_error("Bad ${} expression in inline asm string: '" +
+                         Twine(AsmStr) + "'");
+    ++CurPtr; // Consume '}' character.
+
+    // We get the value number now, let's go on get the operand number.
+    // Scan to find the SDNode operand number for the Val.
+    unsigned OpNo = InlineAsm::Op_FirstOperand;
+    unsigned Flags =
+        cast<ConstantSDNode>(Node->getOperand(OpNo))->getZExtValue();
+    for (; Val; --Val) {
+      if (OpNo >= Node->getNumOperands()) {
+        report_fatal_error("invalid operand in inline asm: '" + Twine(AsmStr) +
+                           "'");
+      }
+      OpNo += InlineAsm::getNumOperandRegisters(Flags) + 1;
+      Flags = cast<ConstantSDNode>(Node->getOperand(OpNo))->getZExtValue();
+    }
+
+    if (InlineAsm::getKind(Flags) != InlineAsm::Kind_Mem)
+      continue;
+
+    // Collect the mem operand ID of call in inline asm.
+    CallOpNos.insert(OpNo);
+  }
+
+  return !CallOpNos.empty();
+}
+
+static GlobalAddressSDNode *getGAFromLoad(SDNode *Node, unsigned OpNo) {
+  SDValue Op = Node->getOperand(OpNo);
+
+  // First make sure Op is a load.
+  const MachineSDNode *MN = dyn_cast<MachineSDNode>(Op.getNode());
+  if (!MN)
+    return nullptr;
+  if (MN->memoperands_empty())
+    return nullptr;
+  MachineSDNode::mmo_iterator I = MN->memoperands_begin();
+  if (!(*I)->isLoad())
+    return nullptr;
+
+  // Make sure the Mem Index is noreg.
+  // TODO, Refine me, maybe no need to check these index for
+  // "call in inline" and "got load" with global address.
+  SDNode *NodeIndex = Node->getOperand(OpNo + 2).getNode();
+  SDNode *OpIndex = Op.getOperand(2).getNode();
+  if (cast<RegisterSDNode>(OpIndex)->getReg() ||
+      cast<RegisterSDNode>(NodeIndex)->getReg())
+    return nullptr;
+
+  // Then make sure the offset is Global Address.
+  SDNode *GA = Op.getOperand(3).getNode();
+  GlobalAddressSDNode *TGA = dyn_cast<GlobalAddressSDNode>(GA);
+  return TGA;
+}
+
 /// EmitSpecialNode - Generate machine code for a target-independent node and
 /// needed dependencies.
-void InstrEmitter::
-EmitSpecialNode(SDNode *Node, bool IsClone, bool IsCloned,
+void InstrEmitter::EmitSpecialNode(SDNode *Node, bool IsClone, bool IsCloned,
                                    DenseMap<SDValue, Register> &VRBaseMap) {
   switch (Node->getOpcode()) {
   default:
 #ifndef NDEBUG
     Node->dump();
 #endif
     llvm_unreachable("This target-independent node should have been selected!");
   case ISD::EntryToken:
     llvm_unreachable("EntryToken should have been excluded from the schedule!");
if (RegisterSDNode *R = dyn_cast<RegisterSDNode>(SrcVal))
       SrcReg = R->getReg();
     else
       SrcReg = getVR(SrcVal, VRBaseMap);
 
     if (SrcReg == DestReg) // Coalesced away the copy? Ignore.
       break;
 
     BuildMI(*MBB, InsertPos, Node->getDebugLoc(), TII->get(TargetOpcode::COPY),
-            DestReg).addReg(SrcReg);
+            DestReg)
+        .addReg(SrcReg);
     break;
   }
   case ISD::CopyFromReg: {
     unsigned SrcReg = cast<RegisterSDNode>(Node->getOperand(1))->getReg();
     EmitCopyFromReg(Node, 0, IsClone, IsCloned, SrcReg, VRBaseMap);
     break;
   }
   case ISD::EH_LABEL:
   case ISD::ANNOTATION_LABEL: {
     unsigned Opc = (Node->getOpcode() == ISD::EH_LABEL)
                        ? TargetOpcode::EH_LABEL
                        : TargetOpcode::ANNOTATION_LABEL;
     MCSymbol *S = cast<LabelSDNode>(Node)->getLabel();
-    BuildMI(*MBB, InsertPos, Node->getDebugLoc(),
-            TII->get(Opc)).addSym(S);
+    BuildMI(*MBB, InsertPos, Node->getDebugLoc(), TII->get(Opc)).addSym(S);
     break;
   }
 
   case ISD::LIFETIME_START:
   case ISD::LIFETIME_END: {
     unsigned TarOp = (Node->getOpcode() == ISD::LIFETIME_START)
                          ? TargetOpcode::LIFETIME_START
                          : TargetOpcode::LIFETIME_END;
     auto *FI = cast<FrameIndexSDNode>(Node->getOperand(1));
     BuildMI(*MBB, InsertPos, Node->getDebugLoc(), TII->get(TarOp))
         .addFrameIndex(FI->getIndex());
     break;
   }
 
   case ISD::PSEUDO_PROBE: {
     unsigned TarOp = TargetOpcode::PSEUDO_PROBE;
     auto Guid = cast<PseudoProbeSDNode>(Node)->getGuid();
     auto Index = cast<PseudoProbeSDNode>(Node)->getIndex();
     auto Attr = cast<PseudoProbeSDNode>(Node)->getAttributes();
 
     BuildMI(*MBB, InsertPos, Node->getDebugLoc(), TII->get(TarOp))
         .addImm(Guid)
         .addImm(Index)
         .addImm((uint8_t)PseudoProbeType::Block)
         .addImm(Attr);
     break;
   }
 
   case ISD::INLINEASM:
   case ISD::INLINEASM_BR: {
     unsigned NumOps = Node->getNumOperands();
-    if (Node->getOperand(NumOps-1).getValueType() == MVT::Glue)
-      --NumOps;  // Ignore the glue operand.
+    if (Node->getOperand(NumOps - 1).getValueType() == MVT::Glue)
+      --NumOps; // Ignore the glue operand.
 
     // Create the inline asm machine instruction.
     unsigned TgtOpc = Node->getOpcode() == ISD::INLINEASM_BR
                           ? TargetOpcode::INLINEASM_BR
                           : TargetOpcode::INLINEASM;
     MachineInstrBuilder MIB =
         BuildMI(*MF, Node->getDebugLoc(), TII->get(TgtOpc));
 
     // Add the asm string as an external symbol operand.
     SDValue AsmStrV = Node->getOperand(InlineAsm::Op_AsmString);
     const char *AsmStr = cast<ExternalSymbolSDNode>(AsmStrV)->getSymbol();
     MIB.addExternalSymbol(AsmStr);
 
     // Add the HasSideEffect, isAlignStack, AsmDialect, MayLoad and MayStore
     // bits.
     int64_t ExtraInfo =
-      cast<ConstantSDNode>(Node->getOperand(InlineAsm::Op_ExtraInfo))->
-                          getZExtValue();
+        cast<ConstantSDNode>(Node->getOperand(InlineAsm::Op_ExtraInfo))
+            ->getZExtValue();
     MIB.addImm(ExtraInfo);
 
     // Remember to operand index of the group flags.
     SmallVector<unsigned, 8> GroupIdx;
 
     // Remember registers that are part of early-clobber defs.
     SmallVector<unsigned, 8> ECRegs;
 
+    // Find if call operand in inine asm need to be replaced.
+    // In Linux PIC model, Global Address (GA) of Global Variable (GV) will be
+    // got from loading GOT slot. Different instructions has different
+    // understanding about using Global Address.
+    //
+    // For example in X86:
+    // 1. We assign a value to GV, we can use "MOV GV, ...", it corresping to
+    // "MOV (Global Address) ...".
+    // 2. We got related address of GV, we can use "LEA GV, ...", it corresping
+    // to "LEA (Global Address) ...".
+    // 3. But if we call the label (GV can be a label), we can use "CALL GV", it
+    // didn't equal with "CALL (Global Address)".
+    //
+    // So, it is obvious that Global Address of Global Variable should be
+    // specially handled case by case. This is done in normal IR/MIR which has
+    // "single" purpose use of the Global Address. But things changed in inline
+    // asm which represented by only one IR/MIR but may contains a lot of
+    // instructions with mult-purpose on same or different Global Address. What
+    // is more, llvm didn't distinguish the instructions in inline asm IR/MIR.
+    //
+    // TODO: The other targets may also has this problem, we need to fix them
+    // too. It is an arch defect for llvm inline asm.
+    bool Replace = false;
+    SmallSet<unsigned, 2> CallOpNos;
+    const TargetMachine &TM = MF->getTarget();
+    if (TM.getTargetTriple().isX86() && TM.isPositionIndependent() &&
+        TM.getTargetTriple().isOSLinux())
+      Replace = getInlineAsmCallOperands(AsmStr, Node, CallOpNos);
+
     // Add all of the operand registers to the instruction.
     for (unsigned i = InlineAsm::Op_FirstOperand; i != NumOps;) {
       unsigned Flags =
           cast<ConstantSDNode>(Node->getOperand(i))->getZExtValue();
       const unsigned NumVals = InlineAsm::getNumOperandRegisters(Flags);
 
       GroupIdx.push_back(MIB->getNumOperands());
       MIB.addImm(Flags);
-      ++i;  // Skip the ID value.
+      ++i; // Skip the ID value.
 
       switch (InlineAsm::getKind(Flags)) {
       default: llvm_unreachable("Bad flags!");
         case InlineAsm::Kind_RegDef:
         for (unsigned j = 0; j != NumVals; ++j, ++i) {
           unsigned Reg = cast<RegisterSDNode>(Node->getOperand(i))->getReg();
           // FIXME: Add dead flags for physical and virtual registers defined.
           // For now, mark physical register defs as implicit to help fast
           // regalloc. This makes inline asm look a lot like calls.
           MIB.addReg(Reg,
                      RegState::Define |
                          getImplRegState(Register::isPhysicalRegister(Reg)));
         }
         break;
       case InlineAsm::Kind_RegDefEarlyClobber:
       case InlineAsm::Kind_Clobber:
         for (unsigned j = 0; j != NumVals; ++j, ++i) {
           unsigned Reg = cast<RegisterSDNode>(Node->getOperand(i))->getReg();
           MIB.addReg(Reg,
                      RegState::Define | RegState::EarlyClobber |
                          getImplRegState(Register::isPhysicalRegister(Reg)));
           ECRegs.push_back(Reg);
         }
         break;
-      case InlineAsm::Kind_RegUse:  // Use of register.
-      case InlineAsm::Kind_Imm:  // Immediate.
-      case InlineAsm::Kind_Mem:  // Addressing mode.
+      case InlineAsm::Kind_RegUse: { // Use of register.
         // The addressing mode has been selected, just add all of the
         // operands to the machine instruction.
         for (unsigned j = 0; j != NumVals; ++j, ++i)
           AddOperand(MIB, Node->getOperand(i), 0, nullptr, VRBaseMap,
                      /*IsDebug=*/false, IsClone, IsCloned);
 
-        // Manually set isTied bits.
-        if (InlineAsm::getKind(Flags) == InlineAsm::Kind_RegUse) {
-          unsigned DefGroup = 0;
-          if (InlineAsm::isUseOperandTiedToDef(Flags, DefGroup)) {
-            unsigned DefIdx = GroupIdx[DefGroup] + 1;
-            unsigned UseIdx = GroupIdx.back() + 1;
-            for (unsigned j = 0; j != NumVals; ++j)
-              MIB->tieOperands(DefIdx + j, UseIdx + j);
-          }
+        break;
+      }
+      case InlineAsm::Kind_Imm: // Immediate.
+        for (unsigned j = 0; j != NumVals; ++j, ++i)
+          AddOperand(MIB, Node->getOperand(i), 0, nullptr, VRBaseMap,
+                     /*IsDebug=*/false, IsClone, IsCloned);
+        break;
+      case InlineAsm::Kind_Mem: { // Addressing mode.
+        bool IsCallOp = CallOpNos.contains(i - 1);
+
+        if (Replace && IsCallOp) {
+          GlobalAddressSDNode *TGA = nullptr;
+          if (TM.getCodeModel() == CodeModel::Large) {
+            // Handle "call func_label" in large code model.
+            // In large code model, funciton label can only be got from load.
+            // let's directly call the *(load address).
+            // TODO: Refine me:
+            // Here assume the index of "inline call func_label" is always
+            // noreg. And here may be no need to check load for "call
+            // func_label", seems func_label must be got from load in large code
+            // model.
+            SDNode *LoadCallAddr = Node->getOperand(i).getNode();
+            const MachineSDNode *MN = dyn_cast<MachineSDNode>(LoadCallAddr);
+            if (MN && !MN->memoperands_empty() &&
+                (*MN->memoperands_begin())->isLoad()) {
+              for (unsigned j = 0; j != NumVals; ++j, ++i) {
+                AddOperand(MIB, LoadCallAddr->getOperand(j), 0, nullptr,
+                           VRBaseMap,
+                           /*IsDebug=*/false, IsClone, IsCloned);
+              }
+              break;
+            }
+          } else {
+            TGA = getGAFromLoad(Node, i);
+          }
+
+          // In PIC model (non-large code model)
+          // We want to print "call func_label" to "call func_label@plt".
+          if (TGA) {
+            MachineInstr *MI = MIB.getInstr();
+            unsigned OldFlagsID = MI->getNumOperands() - 1;
+            unsigned ConstraintID = InlineAsm::getMemoryConstraintID(Flags);
+            unsigned NewFlags = InlineAsm::getFlagWord(InlineAsm::Kind_Mem, 1);
+            NewFlags = InlineAsm::getFlagWordForMem(NewFlags, ConstraintID);
+            MI->RemoveOperand(OldFlagsID);
+            MIB.addImm(NewFlags);
+
+            const GlobalValue *GV = TGA->getGlobal();
+            unsigned char OpFlags =
+                MF->getSubtarget().classifyGlobalFunctionReference(GV);
+            MIB.addGlobalAddress(TGA->getGlobal(), TGA->getOffset(), OpFlags);
+            i += NumVals;
+            break;
+          }
+        }
+
+        for (unsigned j = 0; j != NumVals; ++j, ++i) {
+          AddOperand(MIB, Node->getOperand(i), 0, nullptr, VRBaseMap,
+                     /*IsDebug=*/false, IsClone, IsCloned);
         }
         break;
       }
       }
+    }
 
     // GCC inline assembly allows input operands to also be early-clobber
     // output operands (so long as the operand is written only after it's
     // used), but this does not match the semantics of our early-clobber flag.
     // If an early-clobber operand register is also an input operand register,
     // then remove the early-clobber flag.
     for (unsigned Reg : ECRegs) {
       if (MIB->readsRegister(Reg, TRI)) {

llvm/lib/Target/X86/X86AsmPrinter.cpp

case 'q': // Print SImode register
       break;
     case 'H':
       if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
         return true;  // Unsupported modifier in Intel inline assembly.
       } else {
         PrintMemReference(MI, OpNo, O, "H");
       }
       return false;
-    case 'P': // Don't print @PLT, but do print as memory.
+    case 'P': // Call operand modifer, E.g. omit RIP, add @PLT.
       if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+        if (Subtarget->isPositionIndependent() &&
+            MI->getOperand(OpNo).isGlobal())
+          PrintSymbolOperand(MI->getOperand(OpNo), O);
+        else
           PrintIntelMemReference(MI, OpNo, O, "no-rip");
       } else {
         PrintMemReference(MI, OpNo, O, "no-rip");
       }
       return false;
     }
   }
   if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
     PrintIntelMemReference(MI, OpNo, O, nullptr);

llvm/test/CodeGen/X86/inline-asm-call.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic | FileCheck %s --check-prefix=CHECK-X64
+; RUN: llc < %s -mtriple=i386-unknown-linux-gnu -relocation-model=pic | FileCheck %s --check-prefix=CHECK-X86
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic -code-model=large | FileCheck %s --check-prefix=CHECK-X64-LARGE
+; RUN: llc < %s -mtriple=i386-unknown-linux-gnu -relocation-model=pic -code-model=large | FileCheck %s --check-prefix=CHECK-X86-LARGE
+
+; The tested IR came from following test.c
+; We hope "call sincos" in inline asm should correctly generate the code with
+; PIC model, not "call *(load sincos@GOT)" which is wrong for doing 1 more
+; time load and bad for relocation.
+
+; test.c:
+; extern void sincos();
+; int Arr[10] = {1,};
+; void foo() {
+;  asm {
+;    lea ecx, Arr
+;    lea edx, sincos
+;    call sincos
+;    mov eax, 0
+;    ret }
+; }
+
+; clang t.c -fasm-blocks -S -fpic -emit-llvm test.c
+
+@Arr = global <{ i32, [9 x i32] }> <{ i32 1, [9 x i32] zeroinitializer }>, align 16
+
+define void @foo() {
+; CHECK-X64-LABEL: foo:
+; CHECK-X64:       # %bb.0: # %entry
+; CHECK-X64-NEXT:    movq sincos@GOTPCREL(%rip), %rsi
+; CHECK-X64-NEXT:    movq Arr@GOTPCREL(%rip), %rdi
+; CHECK-X64-NEXT:    #APP
+; CHECK-X64-EMPTY:
+; CHECK-X64-NEXT:    leal (%rdi), %ecx
+; CHECK-X64-NEXT:    leal (%rsi), %edx
+; CHECK-X64-NEXT:    callq sincos@PLT
+; CHECK-X64-NEXT:    movl $0, %eax
+; CHECK-X64-NEXT:    retq
+; CHECK-X64-EMPTY:
+; CHECK-X64-NEXT:    #NO_APP
+; CHECK-X64-NEXT:    retq
+;
+; CHECK-X86-LABEL: foo:
+; CHECK-X86:       # %bb.0: # %entry
+; CHECK-X86-NEXT:    pushl %edi
+; CHECK-X86-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-X86-NEXT:    pushl %esi
+; CHECK-X86-NEXT:    .cfi_def_cfa_offset 12
+; CHECK-X86-NEXT:    .cfi_offset %esi, -12
+; CHECK-X86-NEXT:    .cfi_offset %edi, -8
+; CHECK-X86-NEXT:    calll .L0$pb
+; CHECK-X86-NEXT:    .cfi_adjust_cfa_offset 4
+; CHECK-X86-NEXT:  .L0$pb:
+; CHECK-X86-NEXT:    popl %eax
+; CHECK-X86-NEXT:    .cfi_adjust_cfa_offset -4
+; CHECK-X86-NEXT:  .Ltmp0:
+; CHECK-X86-NEXT:    addl $_GLOBAL_OFFSET_TABLE_+(.Ltmp0-.L0$pb), %eax
+; CHECK-X86-NEXT:    movl sincos@GOT(%eax), %esi
+; CHECK-X86-NEXT:    movl Arr@GOT(%eax), %edi
+; CHECK-X86-NEXT:    #APP
+; CHECK-X86-EMPTY:
+; CHECK-X86-NEXT:    leal (%edi), %ecx
+; CHECK-X86-NEXT:    leal (%esi), %edx
+; CHECK-X86-NEXT:    calll sincos@PLT
+; CHECK-X86-NEXT:    movl $0, %eax
+; CHECK-X86-NEXT:    retl
+; CHECK-X86-EMPTY:
+; CHECK-X86-NEXT:    #NO_APP
+; CHECK-X86-NEXT:    popl %esi
+; CHECK-X86-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-X86-NEXT:    popl %edi
+; CHECK-X86-NEXT:    .cfi_def_cfa_offset 4
+; CHECK-X86-NEXT:    retl
+;
+; CHECK-X64-LARGE-LABEL: foo:
+; CHECK-X64-LARGE:       # %bb.0: # %entry
+; CHECK-X64-LARGE-NEXT:  .L0$pb:
+; CHECK-X64-LARGE-NEXT:    leaq .L0$pb(%rip), %rax
+; CHECK-X64-LARGE-NEXT:    movabsq $_GLOBAL_OFFSET_TABLE_-.L0$pb, %rsi
+; CHECK-X64-LARGE-NEXT:    addq %rax, %rsi
+; CHECK-X64-LARGE-NEXT:    movabsq $sincos@GOT, %r9
+; CHECK-X64-LARGE-NEXT:    movq (%rsi,%r9), %r8
+; CHECK-X64-LARGE-NEXT:    movabsq $Arr@GOT, %rax
+; CHECK-X64-LARGE-NEXT:    movq (%rsi,%rax), %rdi
+; CHECK-X64-LARGE-NEXT:    #APP
+; CHECK-X64-LARGE-EMPTY:
+; CHECK-X64-LARGE-NEXT:    leal (%rdi), %ecx
+; CHECK-X64-LARGE-NEXT:    leal (%r8), %edx
+; CHECK-X64-LARGE-NEXT:    callq *(%rsi,%r9)
+; CHECK-X64-LARGE-NEXT:    movl $0, %eax
+; CHECK-X64-LARGE-NEXT:    retq
+; CHECK-X64-LARGE-EMPTY:
+; CHECK-X64-LARGE-NEXT:    #NO_APP
+; CHECK-X64-LARGE-NEXT:    retq
+;
+; CHECK-X86-LARGE-LABEL: foo:
+; CHECK-X86-LARGE:       # %bb.0: # %entry
+; CHECK-X86-LARGE-NEXT:    pushl %ebx
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-X86-LARGE-NEXT:    pushl %edi
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 12
+; CHECK-X86-LARGE-NEXT:    pushl %esi
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-X86-LARGE-NEXT:    .cfi_offset %esi, -16
+; CHECK-X86-LARGE-NEXT:    .cfi_offset %edi, -12
+; CHECK-X86-LARGE-NEXT:    .cfi_offset %ebx, -8
+; CHECK-X86-LARGE-NEXT:    calll .L0$pb
+; CHECK-X86-LARGE-NEXT:    .cfi_adjust_cfa_offset 4
+; CHECK-X86-LARGE-NEXT:  .L0$pb:
+; CHECK-X86-LARGE-NEXT:    popl %esi
+; CHECK-X86-LARGE-NEXT:    .cfi_adjust_cfa_offset -4
+; CHECK-X86-LARGE-NEXT:  .Ltmp0:
+; CHECK-X86-LARGE-NEXT:    addl $_GLOBAL_OFFSET_TABLE_+(.Ltmp0-.L0$pb), %esi
+; CHECK-X86-LARGE-NEXT:    movl sincos@GOT(%esi), %edi
+; CHECK-X86-LARGE-NEXT:    movl Arr@GOT(%esi), %ebx
+; CHECK-X86-LARGE-NEXT:    #APP
+; CHECK-X86-LARGE-EMPTY:
+; CHECK-X86-LARGE-NEXT:    leal (%ebx), %ecx
+; CHECK-X86-LARGE-NEXT:    leal (%edi), %edx
+; CHECK-X86-LARGE-NEXT:    lcalll *sincos@GOT(%esi)
+; CHECK-X86-LARGE-NEXT:    movl $0, %eax
+; CHECK-X86-LARGE-NEXT:    retl
+; CHECK-X86-LARGE-EMPTY:
+; CHECK-X86-LARGE-NEXT:    #NO_APP
+; CHECK-X86-LARGE-NEXT:    popl %esi
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 12
+; CHECK-X86-LARGE-NEXT:    popl %edi
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-X86-LARGE-NEXT:    popl %ebx
+; CHECK-X86-LARGE-NEXT:    .cfi_def_cfa_offset 4
+; CHECK-X86-LARGE-NEXT:    retl
+entry:
+  call void asm sideeffect inteldialect "lea ecx, $0\0A\09lea edx, qword ptr $1\0A\09call qword ptr ${2:P}\0A\09mov eax, $$0\0A\09ret", "*m,*m,*m,~{eax},~{ecx},~{edx},~{dirflag},~{fpsr},~{flags}"([10 x i32]* bitcast (<{ i32, [9 x i32] }>* @Arr to [10 x i32]*), void (...)* @sincos, void (...)* @sincos)
+  ret void
+}
+
+declare void @sincos(...)