From dc5b614fa9a1c83e8275fcb9c3f78444d0a30514 Mon Sep 17 00:00:00 2001 From: Eric Astor Date: Sun, 22 Dec 2019 00:09:37 -0500 Subject: [PATCH] [ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly. Summary: This is documented as the appropriate template modifier for call operands. Fixes PR44272, and adds a regression test. Also adds support for operand modifiers in Intel-style inline assembly. Reviewers: rnk Reviewed By: rnk Subscribers: merge_guards_bot, hiraditya, cfe-commits, llvm-commits Tags: #clang, #llvm Differential Revision: https://reviews.llvm.org/D71677 --- clang/lib/CodeGen/TargetInfo.cpp | 4 +++ clang/test/CodeGen/mozilla-ms-inline-asm.c | 2 +- clang/test/CodeGen/ms-inline-asm.c | 4 +-- clang/test/CodeGen/ms-inline-asm.cpp | 2 +- llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h | 4 +++ llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h | 2 ++ .../lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp | 37 +++++++++++++++++++--- llvm/lib/MC/MCParser/AsmParser.cpp | 8 ++++- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp | 9 ++++++ llvm/lib/Target/X86/AsmParser/X86Operand.h | 6 +++- llvm/lib/Target/X86/X86AsmPrinter.cpp | 37 +++++++++++++++------- llvm/lib/Target/X86/X86AsmPrinter.h | 2 +- llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll | 18 +++++++++++ 13 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp index 9fd4a5f..6c64006 100644 --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -1187,6 +1187,10 @@ static void rewriteInputConstraintReferences(unsigned FirstIn, if (NumDollars % 2 != 0 && Pos < AsmString.size()) { // We have an operand reference. size_t DigitStart = Pos; + if (AsmString[DigitStart] == '{') { + OS << '{'; + ++DigitStart; + } size_t DigitEnd = AsmString.find_first_not_of("0123456789", DigitStart); if (DigitEnd == std::string::npos) DigitEnd = AsmString.size(); diff --git a/clang/test/CodeGen/mozilla-ms-inline-asm.c b/clang/test/CodeGen/mozilla-ms-inline-asm.c index 0774c8c..210c7f2 100644 --- a/clang/test/CodeGen/mozilla-ms-inline-asm.c +++ b/clang/test/CodeGen/mozilla-ms-inline-asm.c @@ -27,7 +27,7 @@ void invoke(void* that, unsigned methodIndex, // CHECK-SAME: sub esp,eax // CHECK-SAME: mov ecx,esp // CHECK-SAME: push $0 -// CHECK-SAME: call dword ptr $2 +// CHECK-SAME: call dword ptr ${2:P} // CHECK-SAME: {{.*}}__MSASMLABEL_.${:uid}__noparams: // CHECK-SAME: mov ecx,$3 // CHECK-SAME: push ecx diff --git a/clang/test/CodeGen/ms-inline-asm.c b/clang/test/CodeGen/ms-inline-asm.c index 00d408f..0c9b35a 100644 --- a/clang/test/CodeGen/ms-inline-asm.c +++ b/clang/test/CodeGen/ms-inline-asm.c @@ -306,7 +306,7 @@ void t24_helper(void) {} void t24() { __asm call t24_helper // CHECK: t24 -// CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper) +// CHECK: call void asm sideeffect inteldialect "call dword ptr ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper) } void t25() { @@ -689,7 +689,7 @@ void dot_operator(){ void call_clobber() { __asm call t41 // CHECK-LABEL: define void @call_clobber - // CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void (i16)* @t41) + // CHECK: call void asm sideeffect inteldialect "call dword ptr ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void (i16)* @t41) } void xgetbv() { diff --git a/clang/test/CodeGen/ms-inline-asm.cpp b/clang/test/CodeGen/ms-inline-asm.cpp index 039cde9..58796ed 100644 --- a/clang/test/CodeGen/ms-inline-asm.cpp +++ b/clang/test/CodeGen/ms-inline-asm.cpp @@ -109,7 +109,7 @@ void test5() { __asm mov x, eax // CHECK: call void asm sideeffect inteldialect // CHECK-SAME: push $0 - // CHECK-SAME: call dword ptr $2 + // CHECK-SAME: call dword ptr ${2:P} // CHECK-SAME: mov $1, eax // CHECK-SAME: "=*m,=*m,*m,~{esp},~{dirflag},~{fpsr},~{flags}"(i32* %y, i32* %x, i32 (float)* @_ZN2T5IiE6createIfEEiT_) } diff --git a/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h b/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h index 2b6e2aa..c3bb8e5 100644 --- a/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h +++ b/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h @@ -71,6 +71,10 @@ public: /// variable/label? Only valid when parsing MS-style inline assembly. virtual bool needAddressOf() const { return false; } + /// isCallOperand - Is this an operand of an inline-assembly call instruction? + /// Only valid when parsing MS-style inline assembly. + virtual bool isCallOperand() const { return false; } + /// isOffsetOf - Do we need to emit code to get the offset of the variable, /// rather then the value of the variable? Only valid when parsing MS-style /// inline assembly. diff --git a/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h b/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h index e64ab9c..e6bc6b9 100644 --- a/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h +++ b/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h @@ -35,6 +35,7 @@ enum AsmRewriteKind { AOK_Align, // Rewrite align as .align. AOK_EVEN, // Rewrite even as .even. AOK_Emit, // Rewrite _emit as .byte. + AOK_CallInput, // Rewrite in terms of ${N:P}. AOK_Input, // Rewrite in terms of $N. AOK_Output, // Rewrite in terms of $N. AOK_SizeDirective, // Add a sizing directive (e.g., dword ptr). @@ -49,6 +50,7 @@ const char AsmRewritePrecedence [] = { 2, // AOK_EVEN 2, // AOK_Emit 3, // AOK_Input + 3, // AOK_CallInput 3, // AOK_Output 5, // AOK_SizeDirective 1, // AOK_Label diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp index 420df26..a017953 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp @@ -207,11 +207,17 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI, } if (Done) break; + bool HasCurlyBraces = false; + if (*LastEmitted == '{') { // ${variable} + ++LastEmitted; // Consume '{' character. + HasCurlyBraces = true; + } + // If we have ${:foo}, then this is not a real operand reference, it is a // "magic" string reference, just like in .td files. Arrange to call // PrintSpecial. - if (LastEmitted[0] == '{' && LastEmitted[1] == ':') { - LastEmitted += 2; + if (HasCurlyBraces && LastEmitted[0] == ':') { + ++LastEmitted; const char *StrStart = LastEmitted; const char *StrEnd = strchr(StrStart, '}'); if (!StrEnd) @@ -238,6 +244,27 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI, report_fatal_error("Invalid $ operand number in inline asm string: '" + Twine(AsmStr) + "'"); + char Modifier[2] = { 0, 0 }; + + if (HasCurlyBraces) { + // If we have curly braces, check for a modifier character. This + // supports syntax like ${0:u}, which correspond to "%u0" in GCC asm. + if (*LastEmitted == ':') { + ++LastEmitted; // Consume ':' character. + if (*LastEmitted == 0) + report_fatal_error("Bad ${:} expression in inline asm string: '" + + Twine(AsmStr) + "'"); + + Modifier[0] = *LastEmitted; + ++LastEmitted; // Consume modifier character. + } + + if (*LastEmitted != '}') + report_fatal_error("Bad ${} expression in inline asm string: '" + + Twine(AsmStr) + "'"); + ++LastEmitted; // Consume '}' character. + } + // Okay, we finally have a value number. Ask the target to print this // operand! unsigned OpNo = InlineAsm::MIOp_FirstOperand; @@ -262,9 +289,11 @@ static void EmitMSInlineAsmStr(const char *AsmStr, const MachineInstr *MI, ++OpNo; // Skip over the ID number. if (InlineAsm::isMemKind(OpFlags)) { - Error = AP->PrintAsmMemoryOperand(MI, OpNo, /*Modifier*/ nullptr, OS); + Error = AP->PrintAsmMemoryOperand( + MI, OpNo, Modifier[0] ? Modifier : nullptr, OS); } else { - Error = AP->PrintAsmOperand(MI, OpNo, /*Modifier*/ nullptr, OS); + Error = AP->PrintAsmOperand(MI, OpNo, + Modifier[0] ? Modifier : nullptr, OS); } } if (Error) { diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp index 9350e7c..7bbd94d0 100644 --- a/llvm/lib/MC/MCParser/AsmParser.cpp +++ b/llvm/lib/MC/MCParser/AsmParser.cpp @@ -5839,7 +5839,10 @@ bool AsmParser::parseMSInlineAsm( InputDecls.push_back(OpDecl); InputDeclsAddressOf.push_back(Operand.needAddressOf()); InputConstraints.push_back(Operand.getConstraint().str()); - AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size()); + if (Operand.isCallOperand()) + AsmStrRewrites.emplace_back(AOK_CallInput, Start, SymName.size()); + else + AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size()); } } @@ -5929,6 +5932,9 @@ bool AsmParser::parseMSInlineAsm( case AOK_Input: OS << '$' << InputIdx++; break; + case AOK_CallInput: + OS << "${" << InputIdx++ << ":P}"; + break; case AOK_Output: OS << '$' << OutputIdx++; break; diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp index 17ce31f..646b407 100644 --- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp +++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp @@ -2866,6 +2866,15 @@ bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name, } } + // Mark the operands of a call instruction. These need to be handled + // differently when referenced in MS-style inline assembly. + if (Name.startswith("call") || Name.startswith("lcall")) { + for (size_t i = 1; i < Operands.size(); ++i) { + X86Operand &Op = static_cast(*Operands[i]); + Op.setCallOperand(true); + } + } + if (Flags) Operands.push_back(X86Operand::CreatePrefix(Flags, NameLoc, NameLoc)); return false; diff --git a/llvm/lib/Target/X86/AsmParser/X86Operand.h b/llvm/lib/Target/X86/AsmParser/X86Operand.h index 593932e..93626f8 100644 --- a/llvm/lib/Target/X86/AsmParser/X86Operand.h +++ b/llvm/lib/Target/X86/AsmParser/X86Operand.h @@ -36,6 +36,7 @@ struct X86Operand final : public MCParsedAsmOperand { StringRef SymName; void *OpDecl; bool AddressOf; + bool CallOperand; struct TokOp { const char *Data; @@ -77,7 +78,7 @@ struct X86Operand final : public MCParsedAsmOperand { }; X86Operand(KindTy K, SMLoc Start, SMLoc End) - : Kind(K), StartLoc(Start), EndLoc(End) {} + : Kind(K), StartLoc(Start), EndLoc(End), CallOperand(false) {} StringRef getSymName() override { return SymName; } void *getOpDecl() override { return OpDecl; } @@ -286,6 +287,9 @@ struct X86Operand final : public MCParsedAsmOperand { return AddressOf; } + bool isCallOperand() const override { return CallOperand; } + void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; } + bool isMem() const override { return Kind == Memory; } bool isMemUnsized() const { return Kind == Memory && Mem.Size == 0; diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp index de13d59..8c2b901 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.cpp +++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp @@ -337,14 +337,22 @@ void X86AsmPrinter::PrintMemReference(const MachineInstr *MI, unsigned OpNo, PrintLeaMemReference(MI, OpNo, O, Modifier); } + void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI, - unsigned OpNo, raw_ostream &O) { + unsigned OpNo, raw_ostream &O, + const char *Modifier) { const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg); unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm(); const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg); const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp); const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg); + // If we really don't want to print out (rip), don't. + bool HasBaseReg = BaseReg.getReg() != 0; + if (HasBaseReg && Modifier && !strcmp(Modifier, "no-rip") && + BaseReg.getReg() == X86::RIP) + HasBaseReg = false; + // If this has a segment register, print it. if (SegReg.getReg()) { PrintOperand(MI, OpNo + X86::AddrSegmentReg, O); @@ -354,7 +362,7 @@ void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI, O << '['; bool NeedPlus = false; - if (BaseReg.getReg()) { + if (HasBaseReg) { PrintOperand(MI, OpNo + X86::AddrBaseReg, O); NeedPlus = true; } @@ -372,7 +380,7 @@ void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI, PrintOperand(MI, OpNo + X86::AddrDisp, O); } else { int64_t DispVal = DispSpec.getImm(); - if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) { + if (DispVal || (!IndexReg.getReg() && !HasBaseReg)) { if (NeedPlus) { if (DispVal > 0) O << " + "; @@ -525,11 +533,6 @@ bool X86AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo, bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo, const char *ExtraCode, raw_ostream &O) { - if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) { - PrintIntelMemReference(MI, OpNo, O); - return false; - } - if (ExtraCode && ExtraCode[0]) { if (ExtraCode[1] != 0) return true; // Unknown modifier. @@ -543,14 +546,26 @@ bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo, // These only apply to registers, ignore on mem. break; case 'H': - PrintMemReference(MI, OpNo, O, "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. - PrintMemReference(MI, OpNo, O, "no-rip"); + if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) { + PrintIntelMemReference(MI, OpNo, O, "no-rip"); + } else { + PrintMemReference(MI, OpNo, O, "no-rip"); + } return false; } } - PrintMemReference(MI, OpNo, O, nullptr); + if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) { + PrintIntelMemReference(MI, OpNo, O, nullptr); + } else { + PrintMemReference(MI, OpNo, O, nullptr); + } return false; } diff --git a/llvm/lib/Target/X86/X86AsmPrinter.h b/llvm/lib/Target/X86/X86AsmPrinter.h index c5b9d28..ee79401 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.h +++ b/llvm/lib/Target/X86/X86AsmPrinter.h @@ -112,7 +112,7 @@ class LLVM_LIBRARY_VISIBILITY X86AsmPrinter : public AsmPrinter { void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O, const char *Modifier); void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo, - raw_ostream &O); + raw_ostream &O, const char *Modifier); public: X86AsmPrinter(TargetMachine &TM, std::unique_ptr Streamer); diff --git a/llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll b/llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll new file mode 100644 index 0000000..ea66ca3 --- /dev/null +++ b/llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll @@ -0,0 +1,18 @@ +; RUN: llc < %s -mtriple=i686-- | FileCheck %s +; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s + +define void @func() { +entry: + ret void +} + +define void @main() { +entry: + call void asm sideeffect inteldialect "call ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @func) + ret void +; CHECK-LABEL: main: +; CHECK: {{## InlineAsm Start|#APP}} +; CHECK: {{call(l|q) func$}} +; CHECK: {{## InlineAsm End|#NO_APP}} +; CHECK: ret{{l|q}} +} -- 2.7.4