Skip to content

Commit f8d6149

Browse files
committed
ms inline asm: Fix {call,jmp} fptr
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 5717236 commit f8d6149

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
@@ -1144,8 +1144,8 @@ class X86AsmParser : public MCTargetAsmParser {
11441144
bool ParseIntelMemoryOperandSize(unsigned &Size);
11451145
bool CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
11461146
unsigned BaseReg, unsigned IndexReg,
1147-
unsigned Scale, SMLoc Start, SMLoc End,
1148-
unsigned Size, StringRef Identifier,
1147+
unsigned Scale, bool NonAbsMem, SMLoc Start,
1148+
SMLoc End, unsigned Size, StringRef Identifier,
11491149
const InlineAsmIdentifierInfo &Info,
11501150
OperandVector &Operands);
11511151

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

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

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

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

0 commit comments

Comments
 (0)