Skip to content

[X86][MC] Check AdSize16 for 16-bit addressing #99761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 20, 2024
Merged

Conversation

phoebewang
Copy link
Contributor

Fixes: #99735

@phoebewang phoebewang requested review from aaupov and tianqingw July 20, 2024 13:41
@llvmbot llvmbot added backend:X86 mc Machine (object) code labels Jul 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-x86

Author: Phoebe Wang (phoebewang)

Changes

Fixes: #99735


Full diff: https://github.com/llvm/llvm-project/pull/99761.diff

2 Files Affected:

  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp (+4-1)
  • (modified) llvm/test/MC/X86/x86-32-coverage.s (+9-1)
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 3cd0af0c7f546..6553e1cc4a930 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -691,9 +691,12 @@ void X86MCCodeEmitter::emitMemModRMByte(
 
   unsigned BaseRegNo = BaseReg ? getX86RegNum(Base) : -1U;
 
+  bool IsAdSize16 = STI.hasFeature(X86::Is32Bit) &&
+                    (TSFlags & X86II::AdSizeMask) == X86II::AdSize16;
+
   // 16-bit addressing forms of the ModR/M byte have a different encoding for
   // the R/M field and are far more limited in which registers can be used.
-  if (X86_MC::is16BitMemOperand(MI, Op, STI)) {
+  if (IsAdSize16 || X86_MC::is16BitMemOperand(MI, Op, STI)) {
     if (BaseReg) {
       // For 32-bit addressing, the row and column values in Table 2-2 are
       // basically the same. It's AX/CX/DX/BX/SP/BP/SI/DI in that order, with
diff --git a/llvm/test/MC/X86/x86-32-coverage.s b/llvm/test/MC/X86/x86-32-coverage.s
index fbe2714aed263..5475946a9d216 100644
--- a/llvm/test/MC/X86/x86-32-coverage.s
+++ b/llvm/test/MC/X86/x86-32-coverage.s
@@ -10790,7 +10790,7 @@ btcl $4, (%eax)
           movdir64b 485498096, %ecx
 
 // CHECK: movdir64b 485498096, %cx
-// CHECK: # encoding: [0x67,0x66,0x0f,0x38,0xf8,0x0d,0xf0,0x1c,0xf0,0x1c]
+// CHECK: # encoding: [0x67,0x66,0x0f,0x38,0xf8,0x0e,0xf0,0x1c]
           movdir64b 485498096, %cx
 
 // CHECK: movdir64b (%edx), %eax
@@ -10877,6 +10877,10 @@ enqcmd  (%bx,%di), %di
 // CHECK: encoding: [0x67,0xf2,0x0f,0x38,0xf8,0x81,0xc0,0x1f]
 enqcmd  8128(%bx,%di), %ax
 
+// CHECK: enqcmd 485498096, %cx
+// CHECK: encoding: [0x67,0xf2,0x0f,0x38,0xf8,0x0e,0xf0,0x1c]
+enqcmd 485498096, %cx
+
 // CHECK: enqcmds (%bx,%di), %di
 // CHECK: encoding: [0x67,0xf3,0x0f,0x38,0xf8,0x39]
 enqcmds (%bx,%di), %di
@@ -10885,6 +10889,10 @@ enqcmds (%bx,%di), %di
 // CHECK: encoding: [0x67,0xf3,0x0f,0x38,0xf8,0x81,0xc0,0x1f]
 enqcmds 8128(%bx,%di), %ax
 
+// CHECK: enqcmds 485498096, %cx
+// CHECK: encoding: [0x67,0xf3,0x0f,0x38,0xf8,0x0e,0xf0,0x1c]
+enqcmds 485498096, %cx
+
 // CHECK: serialize
 // CHECK: encoding: [0x0f,0x01,0xe8]
 serialize

@phoebewang phoebewang merged commit 6395603 into llvm:main Jul 20, 2024
7 of 10 checks passed
@phoebewang phoebewang deleted the enqcmd branch July 20, 2024 13:59
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 20, 2024

LLVM Buildbot has detected a new failure on builder clang-aarch64-sve-vla running on linaro-g3-04 while building llvm at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/1094

Here is the relevant piece of the build log for the reference:

Step 7 (ninja check 1) failure: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
...
PASS: lit :: shtest-recursive-substitution.py (91360 of 91370)
PASS: lit :: allow-retries.py (91361 of 91370)
PASS: lit :: shtest-external-shell-kill.py (91362 of 91370)
PASS: lit :: discovery.py (91363 of 91370)
PASS: lit :: googletest-timeout.py (91364 of 91370)
PASS: lit :: selecting.py (91365 of 91370)
PASS: lit :: shtest-timeout.py (91366 of 91370)
PASS: lit :: max-time.py (91367 of 91370)
PASS: lit :: shtest-shell.py (91368 of 91370)
PASS: lit :: shtest-define.py (91369 of 91370)
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1690.067624

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Fixes: #99735

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[x86] Assembly Errors
4 participants