Skip to content

Commit c449460

Browse files
authored
ms inline asm: Fix {call,jmp} fptr (#73207)
https://reviews.llvm.org/D151863 (2023-05) removed `BaseReg = BaseReg ? BaseReg : 1` (introduced in commit 175d0ae (2013)) and caused a regression: ensuring a non-zero `BaseReg` was to treat `static void (*fptr)(); __asm { call fptr }` as non-`AbsMem` `AsmOperandClass` and generate `call $0`/`callq *fptr(%rip)` instead of `call ${0:P}`/`callq *fptr` (The asm template argument modifier `P` is for call targets, while no modifier is used by other instructions like `mov rax, fptr`) This patch reinstates the BaseReg-setting statement but places it inside `if (IsGlobalLV)` for clarify. The special case is unfortunate, but we also have special case in similar places (https://reviews.llvm.org/D149920). Fix: #73033
1 parent bf95a0c commit c449460

File tree

2 files changed

+29
-16
lines changed

2 files changed

+29
-16
lines changed

clang/test/CodeGen/ms-inline-asm-64.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,22 @@ int t4(void) {
6060
}
6161

6262
void bar() {}
63+
static void (*fptr)();
6364

6465
void t5(void) {
6566
__asm {
6667
call bar
6768
jmp bar
69+
call fptr
70+
jmp fptr
6871
}
6972
// CHECK: t5
7073
// CHECK: call void asm sideeffect inteldialect
7174
// CHECK-SAME: call ${0:P}
7275
// CHECK-SAME: jmp ${1:P}
73-
// CHECK-SAME: "*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(void (...)) @bar, ptr elementtype(void (...)) @bar)
76+
// CHECK-SAME: call $2
77+
// CHECK-SAME: jmp $3
78+
// CHECK-SAME: "*m,*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(void (...)) @bar, ptr elementtype(void (...)) @bar, ptr elementtype(ptr) @fptr, ptr elementtype(ptr) @fptr)
7479
}
7580

7681
void t47(void) {

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,8 +1143,8 @@ class X86AsmParser : public MCTargetAsmParser {
11431143
bool ParseIntelMemoryOperandSize(unsigned &Size);
11441144
bool CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
11451145
unsigned BaseReg, unsigned IndexReg,
1146-
unsigned Scale, SMLoc Start, SMLoc End,
1147-
unsigned Size, StringRef Identifier,
1146+
unsigned Scale, bool NonAbsMem, SMLoc Start,
1147+
SMLoc End, unsigned Size, StringRef Identifier,
11481148
const InlineAsmIdentifierInfo &Info,
11491149
OperandVector &Operands);
11501150

@@ -1744,10 +1744,13 @@ bool X86AsmParser::parseOperand(OperandVector &Operands, StringRef Name) {
17441744
return parseATTOperand(Operands);
17451745
}
17461746

1747-
bool X86AsmParser::CreateMemForMSInlineAsm(
1748-
unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
1749-
unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
1750-
const InlineAsmIdentifierInfo &Info, OperandVector &Operands) {
1747+
bool X86AsmParser::CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
1748+
unsigned BaseReg, unsigned IndexReg,
1749+
unsigned Scale, bool NonAbsMem,
1750+
SMLoc Start, SMLoc End,
1751+
unsigned Size, StringRef Identifier,
1752+
const InlineAsmIdentifierInfo &Info,
1753+
OperandVector &Operands) {
17511754
// If we found a decl other than a VarDecl, then assume it is a FuncDecl or
17521755
// some other label reference.
17531756
if (Info.isKind(InlineAsmIdentifierInfo::IK_Label)) {
@@ -1772,11 +1775,15 @@ bool X86AsmParser::CreateMemForMSInlineAsm(
17721775
}
17731776
// It is widely common for MS InlineAsm to use a global variable and one/two
17741777
// registers in a mmory expression, and though unaccessible via rip/eip.
1775-
if (IsGlobalLV && (BaseReg || IndexReg)) {
1776-
Operands.push_back(X86Operand::CreateMem(getPointerWidth(), Disp, Start,
1777-
End, Size, Identifier, Decl, 0,
1778-
BaseReg && IndexReg));
1779-
return false;
1778+
if (IsGlobalLV) {
1779+
if (BaseReg || IndexReg) {
1780+
Operands.push_back(X86Operand::CreateMem(getPointerWidth(), Disp, Start,
1781+
End, Size, Identifier, Decl, 0,
1782+
BaseReg && IndexReg));
1783+
return false;
1784+
}
1785+
if (NonAbsMem)
1786+
BaseReg = 1; // Make isAbsMem() false
17801787
}
17811788
Operands.push_back(X86Operand::CreateMem(
17821789
getPointerWidth(), SegReg, Disp, BaseReg, IndexReg, Scale, Start, End,
@@ -2620,18 +2627,19 @@ bool X86AsmParser::parseIntelOperand(OperandVector &Operands, StringRef Name) {
26202627
CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, is64BitMode(),
26212628
ErrMsg))
26222629
return Error(Start, ErrMsg);
2630+
bool IsUnconditionalBranch =
2631+
Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
26232632
if (isParsingMSInlineAsm())
2624-
return CreateMemForMSInlineAsm(RegNo, Disp, BaseReg, IndexReg, Scale, Start,
2625-
End, Size, SM.getSymName(),
2633+
return CreateMemForMSInlineAsm(RegNo, Disp, BaseReg, IndexReg, Scale,
2634+
IsUnconditionalBranch && is64BitMode(),
2635+
Start, End, Size, SM.getSymName(),
26262636
SM.getIdentifierInfo(), Operands);
26272637

26282638
// When parsing x64 MS-style assembly, all non-absolute references to a named
26292639
// variable default to RIP-relative.
26302640
unsigned DefaultBaseReg = X86::NoRegister;
26312641
bool MaybeDirectBranchDest = true;
26322642

2633-
bool IsUnconditionalBranch =
2634-
Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
26352643
if (Parser.isParsingMasm()) {
26362644
if (is64BitMode() && SM.getElementSize() > 0) {
26372645
DefaultBaseReg = X86::RIP;

0 commit comments

Comments
 (0)