Skip to content

[X86][MC] Fix wrong action when encoding enqcmd/enqcmds #77571

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
Jan 11, 2024

Conversation

XinWang10
Copy link
Contributor

@XinWang10 XinWang10 commented Jan 10, 2024

Mentioned in #77293, enqcmd/enqcmds are special for its mem operand, like movdir64b(see 4dd5e9c60efa9), 0x67 prefix can not only modify its address size, so it's mem base and index reg should be the same type as source reg.

@llvmbot llvmbot added backend:X86 mc Machine (object) code labels Jan 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-x86

Author: None (XinWang10)

Changes

enqcmd/enqcmds are special for its mem operand similar to movdir64b(could see 4dd5e9c60efa9), 67 prefex can not only modify its add size,
so it's mem base and index reg should be the same type as source reg, such as
enqcmd(%rdx), rcx, and could not be enqcmd(%edx), rcx.


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrMisc.td (+6-6)
  • (modified) llvm/test/MC/X86/index-operations.s (+58)
diff --git a/llvm/lib/Target/X86/X86InstrMisc.td b/llvm/lib/Target/X86/X86InstrMisc.td
index 97c625a64cfc0b..753cf62392a17b 100644
--- a/llvm/lib/Target/X86/X86InstrMisc.td
+++ b/llvm/lib/Target/X86/X86InstrMisc.td
@@ -1523,28 +1523,28 @@ def MOVDIR64B64_EVEX : I<0xF8, MRMSrcMem, (outs), (ins GR64:$dst, i512mem_GR64:$
 // ENQCMD/S - Enqueue 64-byte command as user with 64-byte write atomicity
 //
 let SchedRW = [WriteStore], Defs = [EFLAGS] in {
-  def ENQCMD16 : I<0xF8, MRMSrcMem, (outs), (ins GR16:$dst, i512mem:$src),
+  def ENQCMD16 : I<0xF8, MRMSrcMem, (outs), (ins GR16:$dst, i512mem_GR16:$src),
                  "enqcmd\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmd GR16:$dst, addr:$src))]>,
                  T8, XD, AdSize16, Requires<[HasENQCMD, Not64BitMode]>;
-  def ENQCMD32 : I<0xF8, MRMSrcMem, (outs), (ins GR32:$dst, i512mem:$src),
+  def ENQCMD32 : I<0xF8, MRMSrcMem, (outs), (ins GR32:$dst, i512mem_GR32:$src),
                  "enqcmd\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmd GR32:$dst, addr:$src))]>,
                  T8, XD, AdSize32, Requires<[HasENQCMD]>;
-  def ENQCMD64 : I<0xF8, MRMSrcMem, (outs), (ins GR64:$dst, i512mem:$src),
+  def ENQCMD64 : I<0xF8, MRMSrcMem, (outs), (ins GR64:$dst, i512mem_GR64:$src),
                  "enqcmd\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmd GR64:$dst, addr:$src))]>,
                  T8, XD, AdSize64, Requires<[HasENQCMD, In64BitMode]>;
 
-  def ENQCMDS16 : I<0xF8, MRMSrcMem, (outs), (ins GR16:$dst, i512mem:$src),
+  def ENQCMDS16 : I<0xF8, MRMSrcMem, (outs), (ins GR16:$dst, i512mem_GR16:$src),
                  "enqcmds\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmds GR16:$dst, addr:$src))]>,
                  T8, XS, AdSize16, Requires<[HasENQCMD, Not64BitMode]>;
-  def ENQCMDS32 : I<0xF8, MRMSrcMem, (outs), (ins GR32:$dst, i512mem:$src),
+  def ENQCMDS32 : I<0xF8, MRMSrcMem, (outs), (ins GR32:$dst, i512mem_GR32:$src),
                  "enqcmds\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmds GR32:$dst, addr:$src))]>,
                  T8, XS, AdSize32, Requires<[HasENQCMD]>;
-  def ENQCMDS64 : I<0xF8, MRMSrcMem, (outs), (ins GR64:$dst, i512mem:$src),
+  def ENQCMDS64 : I<0xF8, MRMSrcMem, (outs), (ins GR64:$dst, i512mem_GR64:$src),
                  "enqcmds\t{$src, $dst|$dst, $src}",
                  [(set EFLAGS, (X86enqcmds GR64:$dst, addr:$src))]>,
                  T8, XS, AdSize64, Requires<[HasENQCMD, In64BitMode]>;
diff --git a/llvm/test/MC/X86/index-operations.s b/llvm/test/MC/X86/index-operations.s
index 899cf4656549f6..59498b0ced1249 100644
--- a/llvm/test/MC/X86/index-operations.s
+++ b/llvm/test/MC/X86/index-operations.s
@@ -188,3 +188,61 @@ movdir64b 291(%esi, %eiz, 4), %ebx
 
 movdir64b 291(%rsi, %riz, 4), %rbx
 // 64: movdir64b 291(%rsi,%riz,4), %rbx # encoding: [0x66,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+
+enqcmd	291(%si), %ecx
+// ERR64: error: invalid 16-bit base register
+// ERR32: invalid operand
+// ERR16: invalid operand
+
+enqcmd	291(%esi), %cx
+// ERR64: error: invalid operand for instruction
+// ERR32: invalid operand
+// ERR16: invalid operand
+
+enqcmd (%rdx), %r15d
+// ERR64: [[#@LINE-1]]:[[#]]: error: invalid operand
+
+enqcmd (%edx), %r15
+// ERR64: [[#@LINE-1]]:[[#]]: error: invalid operand
+
+enqcmd (%eip), %ebx
+// 64: enqcmd (%eip), %ebx # encoding: [0x67,0xf2,0x0f,0x38,0xf8,0x1d,0x00,0x00,0x00,0x00]
+
+enqcmd (%rip), %rbx
+// 64: enqcmd (%rip), %rbx # encoding: [0xf2,0x0f,0x38,0xf8,0x1d,0x00,0x00,0x00,0x00]
+
+enqcmd 291(%esi, %eiz, 4), %ebx
+// 64: enqcmd 291(%esi,%eiz,4), %ebx # encoding: [0x67,0xf2,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+// 32: enqcmd 291(%esi,%eiz,4), %ebx # encoding: [0xf2,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+
+enqcmd 291(%rsi, %riz, 4), %rbx
+// 64: enqcmd 291(%rsi,%riz,4), %rbx # encoding: [0xf2,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+
+enqcmds	291(%si), %ecx
+// ERR64: error: invalid 16-bit base register
+// ERR32: invalid operand
+// ERR16: invalid operand
+
+enqcmds	291(%esi), %cx
+// ERR64: error: invalid operand for instruction
+// ERR32: invalid operand
+// ERR16: invalid operand
+
+enqcmds (%rdx), %r15d
+// ERR64: [[#@LINE-1]]:[[#]]: error: invalid operand
+
+enqcmds (%edx), %r15
+// ERR64: [[#@LINE-1]]:[[#]]: error: invalid operand
+
+enqcmds (%eip), %ebx
+// 64: enqcmds (%eip), %ebx # encoding: [0x67,0xf3,0x0f,0x38,0xf8,0x1d,0x00,0x00,0x00,0x00]
+
+enqcmds (%rip), %rbx
+// 64: enqcmds (%rip), %rbx # encoding: [0xf3,0x0f,0x38,0xf8,0x1d,0x00,0x00,0x00,0x00]
+
+enqcmds 291(%esi, %eiz, 4), %ebx
+// 64: enqcmds 291(%esi,%eiz,4), %ebx # encoding: [0x67,0xf3,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+// 32: enqcmds 291(%esi,%eiz,4), %ebx # encoding: [0xf3,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]
+
+enqcmds 291(%rsi, %riz, 4), %rbx
+// 64: enqcmds 291(%rsi,%riz,4), %rbx # encoding: [0xf3,0x0f,0x38,0xf8,0x9c,0xa6,0x23,0x01,0x00,0x00]

KanRobert
KanRobert approved these changes Jan 11, 2024
Copy link
Contributor

@KanRobert KanRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@KanRobert KanRobert changed the title [X86][MC]Fix wrong action for encode enqcmd/enqcmds [X86][MC] Fix wrong action for encode enqcmd/enqcmds Jan 11, 2024
@KanRobert KanRobert changed the title [X86][MC] Fix wrong action for encode enqcmd/enqcmds [X86][MC] Fix wrong action for encoding enqcmd/enqcmds Jan 11, 2024
@KanRobert KanRobert changed the title [X86][MC] Fix wrong action for encoding enqcmd/enqcmds [X86][MC] Fix wrong action when encoding enqcmd/enqcmds Jan 11, 2024
@XinWang10 XinWang10 merged commit 1bc4cb5 into llvm:main Jan 11, 2024
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Mentioned in llvm#77293,
enqcmd/enqcmds are special for its mem operand, like movdir64b(see
llvm@4dd5e9c60efa9), 0x67 prefix
can not only modify its address size, so it's mem base and index reg
should be the same type as source reg.
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.

3 participants