[llvm] [X86][AsmParser] Support parentheses around scale and index register in Intel syntax (PR #211003)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 21 08:52:04 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: Wiktor Ludwichowski (AlnandoFeronso)
<details>
<summary>Changes</summary>
Old patterns and why they broke
- Decisions matching (CurrState, PrevState) tuples (e.g. CurrState == IES_REGISTER && PrevState != IES_MULTIPLY)
to gate committing a register to Base/Index aren't paren-transparent:
any (...) turns CurrState into IES_RPAREN, so the tuple stops matching and
`[reg + (reg)]`, `[reg + (reg*2)]`, `[reg + 2*(reg)]` fell through the guards.
- Duplicated commit blocks. Recognizing whether a register is Base or Scale was placed in four
handlers (onPlus, onMinus, onRBrac, onRParen), forcing onRParen to replay it just to catch (reg).
- Scale was committed immediately in onInteger; a parenthesized scale like (2) * 4 or 2 * (4)
had nowhere to accumulate.
New patterns
- Pending value. Two pending fields: TmpReg and TmpScale, that store the register/scale which is currently
being processed. Commit happens only at (+, -, ]), therefore the solution is parenthesis-independent and
can handle more scale expressions.
- Identical in onPlus/onMinus/onRBrac: !BaseReg && !TmpScale → BaseReg (unscaled);
otherwise → IndexReg with Scale. onRParen no longer commits, parens are purely an infix-calculator concern.
- Multiplicative scale accumulator onInteger. onStar on IES_INTEGER seeds it from the popped operand;
reset at +, -, [, ]. Handles 2 * 4, (2) * 4, 2 * (4), 2 * 2 * 2, (2 * (2 * rbx)) * 2, etc.
- Conflict detection. Pending state makes two invariants diagnosable: TmpReg already set when a
new register arrives via * or ( means multiplication of two registers; NegativeAdditiveTerm
at commit means negative scale. Both previously misassembled silently.
---
Full diff: https://github.com/llvm/llvm-project/pull/211003.diff
3 Files Affected:
- (modified) llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp (+90-66)
- (added) llvm/test/MC/X86/intel-syntax-scale-register-parentheses-error.s (+23)
- (added) llvm/test/MC/X86/intel-syntax-scale-register-parentheses.s (+75)
``````````diff
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 54edcba34a7e9..cbd66c6808749 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -45,6 +45,7 @@
#include <algorithm>
#include <cstdint>
#include <memory>
+#include <optional>
using namespace llvm;
@@ -445,6 +446,7 @@ class X86AsmParser : public MCTargetAsmParser {
IntelExprState State = IES_INIT, PrevState = IES_ERROR;
MCRegister BaseReg, IndexReg, TmpReg;
unsigned Scale = 0;
+ std::optional<unsigned> TmpScale = {};
int64_t Imm = 0;
const MCExpr *Sym = nullptr;
StringRef SymName;
@@ -700,20 +702,27 @@ class X86AsmParser : public MCTargetAsmParser {
IC.pushOperator(IC_PLUS);
NegativeAdditiveTerm = false;
NegativeAdditiveTermLoc = SMLoc();
- if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
- // If we already have a BaseReg, then assume this is the IndexReg with
- // no explicit scale.
- if (!BaseReg) {
+ if (TmpReg) {
+ // A pending scale forces this to be the IndexReg; otherwise a free
+ // BaseReg takes it as an unscaled base.
+ if (!BaseReg && !TmpScale.has_value()) {
BaseReg = TmpReg;
+ TmpReg = MCRegister::NoRegister;
} else {
if (IndexReg)
return regsUseUpError(ErrMsg);
IndexReg = TmpReg;
- Scale = 0;
+ TmpReg = MCRegister::NoRegister;
+ if (TmpScale.has_value() && checkScale(TmpScale.value(), ErrMsg)) {
+ return true;
+ }
+ Scale = TmpScale.value_or(0);
}
}
break;
}
+ // A '+' ends the current additive term, so clear the pending scale.
+ TmpScale.reset();
PrevState = CurrState;
return false;
}
@@ -761,20 +770,27 @@ class X86AsmParser : public MCTargetAsmParser {
return true;
} else
IC.pushOperator(IC_NEG);
- if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
- // If we already have a BaseReg, then assume this is the IndexReg with
- // no explicit scale.
- if (!BaseReg) {
+ if (TmpReg) {
+ // A pending scale forces this to be the IndexReg; otherwise a free
+ // BaseReg takes it as an unscaled base.
+ if (!BaseReg && !TmpScale.has_value()) {
BaseReg = TmpReg;
+ TmpReg = MCRegister::NoRegister;
} else {
if (IndexReg)
return regsUseUpError(ErrMsg);
IndexReg = TmpReg;
- Scale = 0;
+ TmpReg = MCRegister::NoRegister;
+ if (TmpScale.has_value() && checkScale(TmpScale.value(), ErrMsg)) {
+ return true;
+ }
+ Scale = TmpScale.value_or(0);
}
}
break;
}
+ // A '-' ends the current additive term, so clear the pending scale.
+ TmpScale.reset();
PrevState = CurrState;
return false;
}
@@ -816,33 +832,42 @@ class X86AsmParser : public MCTargetAsmParser {
default:
State = IES_ERROR;
break;
- case IES_PLUS:
case IES_MINUS:
- case IES_LPAREN:
+ case IES_PLUS:
case IES_LBRAC:
State = IES_REGISTER;
TmpReg = Reg;
IC.pushOperand(IC_REGISTER);
+ if (NegativeAdditiveTerm) {
+ ErrMsg = "Scale can't be negative";
+ return true;
+ }
break;
+ case IES_LPAREN:
case IES_MULTIPLY:
- // Index Register - Scale * Register
- if (PrevState == IES_INTEGER) {
+ // A register already held in TmpReg means we are multiplying two reg
+ if (TmpReg) {
+ ErrMsg = "Register can't be multiplied with register!";
+ return true;
+ }
+ State = IES_REGISTER;
+ TmpReg = Reg;
+ // Recognize this register as a scaled index register. This covers
+ // 'scale * reg' and 'scale * (reg)', including parenthesized or
+ // multi-factor scales where the accumulated value is held in TmpScale.
+ if (TmpScale.has_value() || PrevState == IES_MULTIPLY ||
+ (PrevState == IES_INTEGER && CurrState == IES_MULTIPLY)) {
if (IndexReg)
return regsUseUpError(ErrMsg);
if (NegativeAdditiveTerm) {
ErrMsg = "Scale can't be negative";
return true;
}
- State = IES_REGISTER;
- IndexReg = Reg;
- // Get the scale and replace the 'Scale * Register' with '0'.
- Scale = IC.popOperand();
- if (checkScale(Scale, ErrMsg))
- return true;
+ // Push an immediate, not the register, so the infix calculator
+ // won't evaluate reg * int; this is a scaled index reg.
IC.pushOperand(IC_IMM);
- IC.popOperator();
} else {
- State = IES_ERROR;
+ IC.pushOperand(IC_REGISTER);
}
break;
}
@@ -911,23 +936,16 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_INIT:
case IES_LBRAC:
State = IES_INTEGER;
- if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
- // Index Register - Register * Scale
- if (IndexReg)
- return regsUseUpError(ErrMsg);
- if (NegativeAdditiveTerm) {
- ErrMsg = "Scale can't be negative";
- return true;
- }
- IndexReg = TmpReg;
- Scale = TmpInt;
- if (checkScale(Scale, ErrMsg))
- return true;
- // Get the scale and replace the 'Register * Scale' with '0'.
- IC.popOperator();
+ // Accumulate the scale: multiply into a pending scale or seed it.
+ if (TmpScale.has_value()) {
+ TmpScale.value() *= TmpInt;
} else {
- IC.pushOperand(IC_IMM, TmpInt);
+ TmpScale = TmpInt;
}
+ // Once an index register is pending, check if TmpScale is valid.
+ if (TmpReg && checkScale(TmpScale.value(), ErrMsg))
+ return true;
+ IC.pushOperand(IC_IMM, TmpInt);
break;
}
PrevState = CurrState;
@@ -940,10 +958,26 @@ class X86AsmParser : public MCTargetAsmParser {
State = IES_ERROR;
break;
case IES_INTEGER:
+ // Seed the scale accumulator with the integer just parsed (the left
+ // operand of this '*'), reading it back from the operand stack.
+ if (!TmpScale.has_value()) {
+ TmpScale = IC.popOperand();
+ IC.pushOperand(IC_IMM, TmpScale.value());
+ }
+ State = IES_MULTIPLY;
+ IC.pushOperator(IC_MULTIPLY);
+ break;
case IES_REGISTER:
case IES_RPAREN:
- State = IES_MULTIPLY;
+ // A register before '*' is a scaled index register. If no scale is
+ // pending yet, replace its operand-stack entry with an immediate so
+ // the infix calculator does not evaluate a reg * int product.
+ if (TmpReg && (!TmpScale.has_value())) {
+ IC.popOperand();
+ IC.pushOperand(IC_IMM);
+ }
IC.pushOperator(IC_MULTIPLY);
+ State = IES_MULTIPLY;
break;
}
}
@@ -993,6 +1027,8 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_CAST:
assert(!BracCount && "BracCount should be zero on parsing's start");
State = IES_LBRAC;
+ // Entering a new memory expression; clear the pending scale.
+ TmpScale.reset();
break;
}
MemExpr = true;
@@ -1015,26 +1051,34 @@ class X86AsmParser : public MCTargetAsmParser {
return true;
}
State = IES_RBRAC;
- if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
- // If we already have a BaseReg, then assume this is the IndexReg with
- // no explicit scale.
- if (!BaseReg) {
+
+ if (TmpReg) {
+ // A pending scale forces this to be the IndexReg; otherwise a free
+ // BaseReg takes it as an unscaled base.
+ if (!BaseReg && !TmpScale.has_value()) {
BaseReg = TmpReg;
- } else {
- if (IndexReg)
- return regsUseUpError(ErrMsg);
+ TmpReg = MCRegister::NoRegister;
+ } else if (!IndexReg) {
if (NegativeAdditiveTerm) {
ErrMsg = "Scale can't be negative";
return true;
}
IndexReg = TmpReg;
- Scale = 0;
+ TmpReg = MCRegister::NoRegister;
+ if (TmpScale.has_value() && checkScale(TmpScale.value(), ErrMsg)) {
+ return true;
+ }
+ Scale = TmpScale.value_or(0);
+ } else {
+ return regsUseUpError(ErrMsg);
}
}
NegativeAdditiveTerm = false;
NegativeAdditiveTermLoc = SMLoc();
break;
}
+ // Leaving the memory expression; clear the pending scale.
+ TmpScale.reset();
PrevState = CurrState;
return false;
}
@@ -1082,26 +1126,6 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_RBRAC:
case IES_RPAREN:
State = IES_RPAREN;
- // In the case of a multiply, onRegister has already set IndexReg
- // directly, with appropriate scale.
- // Otherwise if we just saw a register it has only been stored in
- // TmpReg, so we need to store it into the state machine.
- if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
- // If we already have a BaseReg, then assume this is the IndexReg with
- // no explicit scale.
- if (!BaseReg) {
- BaseReg = TmpReg;
- } else {
- if (IndexReg)
- return regsUseUpError(ErrMsg);
- if (NegativeAdditiveTerm) {
- ErrMsg = "Scale can't be negative";
- return true;
- }
- IndexReg = TmpReg;
- Scale = 0;
- }
- }
IC.pushOperator(IC_RPAREN);
break;
}
diff --git a/llvm/test/MC/X86/intel-syntax-scale-register-parentheses-error.s b/llvm/test/MC/X86/intel-syntax-scale-register-parentheses-error.s
new file mode 100644
index 0000000000000..b0adcfd170c17
--- /dev/null
+++ b/llvm/test/MC/X86/intel-syntax-scale-register-parentheses-error.s
@@ -0,0 +1,23 @@
+// RUN: not llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s 2>&1 | FileCheck %s
+
+// CHECK: error: Scale can't be negative
+ xor [rsi - rdx + 40], eax
+// CHECK: error: Scale can't be negative
+ xor [rsi - 8 * rdx], eax
+// CHECK: error: BaseReg/IndexReg already set!
+ xor [rsi + rbx + rbx], eax
+// CHECK: error: BaseReg/IndexReg already set!
+ xor [rsi + 4*rbx + rbx], eax
+// CHECK: error: BaseReg/IndexReg already set!
+ xor [rsi + (4*rbx) + rbx], eax
+// CHECK: error: Register can't be multiplied with register!
+ xor [rsi + (rax*rbx)], eax
+// CHECK: error: Register can't be multiplied with register!
+ xor [rsi + rax*(rbx)], eax
+// CHECK: error: Register can't be multiplied with register!
+ xor [rsi + rax*rbx], eax
+// CHECK: error: scale factor in address must be 1, 2, 4 or 8
+ xor [rsi + 0*(rbx)], eax
+// CHECK: error: scale factor in address must be 1, 2, 4 or 8
+ xor [rsi + (rbx)*0], eax
+
diff --git a/llvm/test/MC/X86/intel-syntax-scale-register-parentheses.s b/llvm/test/MC/X86/intel-syntax-scale-register-parentheses.s
new file mode 100644
index 0000000000000..5c410e4a02e86
--- /dev/null
+++ b/llvm/test/MC/X86/intel-syntax-scale-register-parentheses.s
@@ -0,0 +1,75 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel --output-asm-variant=1 %s | FileCheck %s
+
+// Test that parentheses around scale register are interpreted correctly
+// Tests parentheses around registers and scale values in register addressing
+// CHECK: xor dword ptr [rsi + rbx + 16], eax
+ xor [rsi + 2 * 8 + (((rbx)))], eax
+// CHECK: xor dword ptr [rsi + rbx + 16], eax
+ xor [rsi + 2 * 8 + (rbx)], eax
+// CHECK: xor dword ptr [rsi + 16], eax
+ xor [rsi + 2 * 8], eax
+// CHECK: xor dword ptr [rsi + 16], eax
+ xor [rsi + (2) * 8], eax
+// CHECK: xor dword ptr [rsi + 16], eax
+ xor [rsi + 2 * (8)], eax
+// CHECK: xor dword ptr [rsi + 2*rbx + 16], eax
+ xor [rsi + (2) * 8 + rbx * 2], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 16], eax
+ xor [rsi + 2 * (8) + rbx * 4], eax
+// CHECK: xor dword ptr [rsi + 2*rbx + 16], eax
+ xor [rsi + 2 * 8 + (((rbx))) * 2], eax
+// CHECK: xor dword ptr [rsi + 8*rbx + 16], eax
+ xor [(rsi) + 2 * 8 + (rbx) * 8], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 16], eax
+ xor [rsi + 2 * 8 + (rbx * 4)], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 40], eax
+ xor [rsi + 2 * 8 + (rbx * 4) + 24], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 16], eax
+ xor [(rsi) + 2 * 8 + (rbx * 4)], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 16], eax
+ xor [(((rsi))) + 2 * 8 + (rbx * 4)], eax
+// CHECK: xor dword ptr [rsi + 4*rbx + 16], eax
+ xor [(((rsi))) + 2 * 8 + (rbx) * 4], eax
+// CHECK: xor dword ptr [rsi + 2*rbx], eax
+ xor [rsi + 2*(rbx)], eax
+// CHECK: xor dword ptr [rsi], eax
+ xor [rsi + 0], eax
+// CHECK: xor dword ptr [rsi], eax
+ xor [0 + rsi], eax
+// CHECK: xor dword ptr [rsi + 2*rbx], eax
+ xor [rsi + (2*rbx)], eax
+// CHECK: xor dword ptr [rsi + 2*rbx - 40], eax
+ xor [rsi + (2*rbx) - 40], eax
+// CHECK: xor dword ptr [rsi - 40], eax
+ xor [rsi - 40], eax
+// CHECK: xor dword ptr [rsi + rbx - 40], eax
+ xor [rsi + rbx - 40], eax
+// CHECK: xor dword ptr [rsi + 2*rbx - 40], eax
+ xor [rsi -40 +(2*rbx)], eax
+// CHECK: xor dword ptr [rsi + 2*rbx - 40], eax
+ xor [rsi -40 +2*(rbx)], eax
+// CHECK: xor dword ptr [rsi + 4*rbx], eax
+ xor [rsi + (4*(rbx))], eax
+// CHECK: xor dword ptr [rsi + 4*rbx - 40], eax
+ xor [rsi + 2*(((2*((rbx))))) - 40], eax
+// CHECK: xor dword ptr [rsi + 4*rbx - 40], eax
+ xor [rsi -40 + 2*(((2*((rbx)))))], eax
+// CHECK: xor dword ptr [rsi + 8*rbx], eax
+ xor [rsi + 2*(2*2*(rbx))], eax
+// CHECK: xor dword ptr [rsi + 2*rbx - 40], eax
+ xor [rsi -40 +2*(rbx)], eax
+// CHECK: xor dword ptr [rsi + 2*rbx - 40], eax
+ xor [rsi -40 +2*((((((rbx))))))], eax
+// CHECK: xor dword ptr [rsi + 4*rbx], eax
+ xor [rsi + (4*(rbx))], eax
+// CHECK: xor dword ptr [rsi + 4*rbx - 40], eax
+ xor [rsi -40 +((((((rbx)))*2))*2)], eax
+// CHECK: xor dword ptr [rsi + 8*rbx - 40], eax
+ xor [rsi -40 +((((((rbx*2))*2)))*2)], eax
+// CHECK: xor dword ptr [rsi + 8*rbx - 40], eax
+ xor [rsi -40 +(((2*(((rbx)))*2))*2)], eax
+// CHECK: xor dword ptr [rsi + 8*rbx - 40], eax
+ xor [rsi -40 +((2*((2*((rbx*2)))))*(1))], eax
+// CHECK: xor dword ptr [rsi + 8*rbx - 40], eax
+ xor [rsi -40 +((2*((1*((rbx*2)))))*(2))], eax
+
``````````
</details>
https://github.com/llvm/llvm-project/pull/211003
More information about the llvm-commits
mailing list