Skip to content

release/19.x: [X86][APX] Fix wrong encoding of promoted KMOV instructions due to missing NoCD8 (#109759) #109767

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 2 commits into from
Oct 1, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Sep 24, 2024

Backport 0d334d8 70529b2

Requested by: @phoebewang

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Sep 24, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Sep 24, 2024

@KanRobert @KanRobert What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Sep 24, 2024

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-mc

Author: None (llvmbot)

Changes

Backport 0d334d8 70529b2

Requested by: @phoebewang


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

4 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrAVX512.td (+2-2)
  • (modified) llvm/test/MC/Disassembler/X86/apx/kmov.txt (+16)
  • (modified) llvm/test/MC/X86/apx/kmov-att.s (+13-1)
  • (modified) llvm/test/MC/X86/apx/kmov-intel.s (+12)
diff --git a/llvm/lib/Target/X86/X86InstrAVX512.td b/llvm/lib/Target/X86/X86InstrAVX512.td
index da690aea43f5c0..cc1f9090c11acc 100644
--- a/llvm/lib/Target/X86/X86InstrAVX512.td
+++ b/llvm/lib/Target/X86/X86InstrAVX512.td
@@ -2625,11 +2625,11 @@ multiclass avx512_mask_mov<bits<8> opc_kk, bits<8> opc_km, bits<8> opc_mk,
   def km#Suffix : I<opc_km, MRMSrcMem, (outs KRC:$dst), (ins x86memop:$src),
                     !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"),
                     [(set KRC:$dst, (vvt (load addr:$src)))]>,
-                  Sched<[WriteLoad]>;
+                  Sched<[WriteLoad]>, NoCD8;
   def mk#Suffix : I<opc_mk, MRMDestMem, (outs), (ins x86memop:$dst, KRC:$src),
                     !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"),
                     [(store KRC:$src, addr:$dst)]>,
-                  Sched<[WriteStore]>;
+                  Sched<[WriteStore]>, NoCD8;
 }
 
 multiclass avx512_mask_mov_gpr<bits<8> opc_kr, bits<8> opc_rk,
diff --git a/llvm/test/MC/Disassembler/X86/apx/kmov.txt b/llvm/test/MC/Disassembler/X86/apx/kmov.txt
index 5d947ff39f2314..ba77dda64e59f5 100644
--- a/llvm/test/MC/Disassembler/X86/apx/kmov.txt
+++ b/llvm/test/MC/Disassembler/X86/apx/kmov.txt
@@ -17,6 +17,22 @@
 # INTEL: {evex} kmovq	k2, k1
 0x62,0xf1,0xfc,0x08,0x90,0xd1
 
+# ATT:   kmovb	-16(%rax), %k0
+# INTEL: kmovb	k0, byte ptr [rax - 16]
+0x62,0xf1,0x7d,0x08,0x90,0x40,0xf0
+
+# ATT:   kmovw	-16(%rax), %k0
+# INTEL: kmovw	k0, word ptr [rax - 16]
+0x62,0xf1,0x7c,0x08,0x90,0x40,0xf0
+
+# ATT:   kmovd	-16(%rax), %k0
+# INTEL: kmovd	k0, dword ptr [rax - 16]
+0x62,0xf1,0xfd,0x08,0x90,0x40,0xf0
+
+# ATT:   kmovq	-16(%rax), %k0
+# INTEL: kmovq	k0, qword ptr [rax - 16]
+0x62,0xf1,0xfc,0x08,0x90,0x40,0xf0
+
 # ATT-NOT: {evex}
 # INTEL-NOT: {evex}
 
diff --git a/llvm/test/MC/X86/apx/kmov-att.s b/llvm/test/MC/X86/apx/kmov-att.s
index 949ef65be98d4c..5f59e0a505b235 100644
--- a/llvm/test/MC/X86/apx/kmov-att.s
+++ b/llvm/test/MC/X86/apx/kmov-att.s
@@ -1,7 +1,7 @@
 # RUN: llvm-mc -triple x86_64 -show-encoding %s | FileCheck %s
 # RUN: not llvm-mc -triple i386 -show-encoding %s 2>&1 | FileCheck %s --check-prefix=ERROR
 
-# ERROR-COUNT-20: error:
+# ERROR-COUNT-24: error:
 # ERROR-NOT: error:
 # CHECK: {evex}	kmovb	%k1, %k2
 # CHECK: encoding: [0x62,0xf1,0x7d,0x08,0x90,0xd1]
@@ -15,6 +15,18 @@
 # CHECK: {evex}	kmovq	%k1, %k2
 # CHECK: encoding: [0x62,0xf1,0xfc,0x08,0x90,0xd1]
          {evex}	kmovq	%k1, %k2
+# CHECK: {evex} kmovb   -16(%rax), %k0
+# CHECK: encoding: [0x62,0xf1,0x7d,0x08,0x90,0x40,0xf0]
+         {evex} kmovb   -0x10(%rax), %k0
+# CHECK: {evex} kmovw   -16(%rax), %k0
+# CHECK: encoding: [0x62,0xf1,0x7c,0x08,0x90,0x40,0xf0]
+         {evex} kmovw   -0x10(%rax), %k0
+# CHECK: {evex} kmovd   -16(%rax), %k0
+# CHECK: encoding: [0x62,0xf1,0xfd,0x08,0x90,0x40,0xf0]
+         {evex} kmovd   -0x10(%rax), %k0
+# CHECK: {evex} kmovq   -16(%rax), %k0
+# CHECK: encoding: [0x62,0xf1,0xfc,0x08,0x90,0x40,0xf0]
+         {evex} kmovq   -0x10(%rax), %k0
 
 # CHECK-NOT: {evex}
 
diff --git a/llvm/test/MC/X86/apx/kmov-intel.s b/llvm/test/MC/X86/apx/kmov-intel.s
index 0cdbd310062eba..51cec67caf9a04 100644
--- a/llvm/test/MC/X86/apx/kmov-intel.s
+++ b/llvm/test/MC/X86/apx/kmov-intel.s
@@ -12,6 +12,18 @@
 # CHECK: {evex}	kmovq	k2, k1
 # CHECK: encoding: [0x62,0xf1,0xfc,0x08,0x90,0xd1]
          {evex}	kmovq	k2, k1
+# CHECK: {evex} kmovb   k0, byte ptr [rax - 16]
+# CHECK: encoding: [0x62,0xf1,0x7d,0x08,0x90,0x40,0xf0]
+         {evex} kmovb   k0, byte ptr [rax - 0x10]
+# CHECK: {evex} kmovw   k0, word ptr [rax - 16]
+# CHECK: encoding: [0x62,0xf1,0x7c,0x08,0x90,0x40,0xf0]
+         {evex} kmovw   k0, word ptr [rax - 0x10]
+# CHECK: {evex} kmovd   k0, dword ptr [rax - 16]
+# CHECK: encoding: [0x62,0xf1,0xfd,0x08,0x90,0x40,0xf0]
+         {evex} kmovd   k0, dword ptr [rax - 0x10]
+# CHECK: {evex} kmovq   k0, qword ptr [rax - 16]
+# CHECK: encoding: [0x62,0xf1,0xfc,0x08,0x90,0x40,0xf0]
+         {evex} kmovq   k0, qword ptr [rax - 0x10]
 
 # CHECK-NOT: {evex}
 

@phoebewang phoebewang changed the title release/19.x: [X86][APX] Do not emit {evex} prefix for memory variant (#109759) release/19.x: [X86][APX] Fix wrong encoding of promoted KMOV instructions due to missing NoCD8 (#109759) Sep 24, 2024
@KanRobert
Copy link
Contributor

@KanRobert @KanRobert What do you think about merging this PR to the release branch?

LGTM

…ssing NoCD8 (llvm#109579)

Promoted KMOV* was encoded with CD8 incorrectly, see
https://godbolt.org/z/cax513hG1

(cherry picked from commit 0d334d8)
This was mistakely changed by llvm#109579, which doesn't match with other
EVEX decoding.

(cherry picked from commit 70529b2)
@tru tru merged commit d401987 into llvm:release/19.x Oct 1, 2024
8 of 10 checks passed
Copy link

github-actions bot commented Oct 1, 2024

@phoebewang (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@phoebewang
Copy link
Contributor

Thanks @tru! The title should be good as the description.

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
Development

Successfully merging this pull request may close these issues.

4 participants