Skip to content

[X86][APX] Fix wrong encoding of promoted KMOV instructions due to missing NoCD8 #109579

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 23, 2024
Merged

[X86][APX] Fix wrong encoding of promoted KMOV instructions due to missing NoCD8 #109579

merged 1 commit into from
Sep 23, 2024

Conversation

phoebewang
Copy link
Contributor

@phoebewang phoebewang commented Sep 22, 2024 •
edited
Loading

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

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

llvmbot commented Sep 22, 2024

@llvm/pr-subscribers-backend-x86

Author: Phoebe Wang (phoebewang)

Changes

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

4 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrAVX512.td (+14-13)
  • (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 928abac46da866..417f31a3516e26 100644
--- a/llvm/lib/Target/X86/X86InstrAVX512.td
+++ b/llvm/lib/Target/X86/X86InstrAVX512.td
@@ -2617,19 +2617,20 @@ defm VFPCLASS : avx512_fp_fpclass_all<"vfpclass", 0x66, 0x67, SchedWriteFCmp>, E
 multiclass avx512_mask_mov<bits<8> opc_kk, bits<8> opc_km, bits<8> opc_mk,
                           string OpcodeStr, RegisterClass KRC, ValueType vvt,
                           X86MemOperand x86memop, string Suffix = ""> {
-  let isMoveReg = 1, hasSideEffects = 0, SchedRW = [WriteMove],
-      explicitOpPrefix = !if(!eq(Suffix, ""), NoExplicitOpPrefix, ExplicitEVEX) in
-  def kk#Suffix : I<opc_kk, MRMSrcReg, (outs KRC:$dst), (ins KRC:$src),
-                    !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"), []>,
-                  Sched<[WriteMove]>;
-  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]>;
-  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]>;
+  let explicitOpPrefix = !if(!eq(Suffix, ""), NoExplicitOpPrefix, ExplicitEVEX) in {
+    let isMoveReg = 1, hasSideEffects = 0, SchedRW = [WriteMove] in
+    def kk#Suffix : I<opc_kk, MRMSrcReg, (outs KRC:$dst), (ins KRC:$src),
+                      !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"), []>,
+                    Sched<[WriteMove]>;
+    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]>, 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]>, 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..45fedbd0da587b 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:   {evex} kmovb   -16(%rax), %k0
+# INTEL: {evex} kmovb   k0, byte ptr [rax - 16]
+0x62,0xf1,0x7d,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovw   -16(%rax), %k0
+# INTEL: {evex} kmovw   k0, word ptr [rax - 16]
+0x62,0xf1,0x7c,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovd   -16(%rax), %k0
+# INTEL: {evex} kmovd   k0, dword ptr [rax - 16]
+0x62,0xf1,0xfd,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovq   -16(%rax), %k0
+# INTEL: {evex} 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}

@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2024

@llvm/pr-subscribers-mc

Author: Phoebe Wang (phoebewang)

Changes

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

4 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrAVX512.td (+14-13)
  • (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 928abac46da866..417f31a3516e26 100644
--- a/llvm/lib/Target/X86/X86InstrAVX512.td
+++ b/llvm/lib/Target/X86/X86InstrAVX512.td
@@ -2617,19 +2617,20 @@ defm VFPCLASS : avx512_fp_fpclass_all<"vfpclass", 0x66, 0x67, SchedWriteFCmp>, E
 multiclass avx512_mask_mov<bits<8> opc_kk, bits<8> opc_km, bits<8> opc_mk,
                           string OpcodeStr, RegisterClass KRC, ValueType vvt,
                           X86MemOperand x86memop, string Suffix = ""> {
-  let isMoveReg = 1, hasSideEffects = 0, SchedRW = [WriteMove],
-      explicitOpPrefix = !if(!eq(Suffix, ""), NoExplicitOpPrefix, ExplicitEVEX) in
-  def kk#Suffix : I<opc_kk, MRMSrcReg, (outs KRC:$dst), (ins KRC:$src),
-                    !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"), []>,
-                  Sched<[WriteMove]>;
-  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]>;
-  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]>;
+  let explicitOpPrefix = !if(!eq(Suffix, ""), NoExplicitOpPrefix, ExplicitEVEX) in {
+    let isMoveReg = 1, hasSideEffects = 0, SchedRW = [WriteMove] in
+    def kk#Suffix : I<opc_kk, MRMSrcReg, (outs KRC:$dst), (ins KRC:$src),
+                      !strconcat(OpcodeStr, "\t{$src, $dst|$dst, $src}"), []>,
+                    Sched<[WriteMove]>;
+    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]>, 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]>, 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..45fedbd0da587b 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:   {evex} kmovb   -16(%rax), %k0
+# INTEL: {evex} kmovb   k0, byte ptr [rax - 16]
+0x62,0xf1,0x7d,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovw   -16(%rax), %k0
+# INTEL: {evex} kmovw   k0, word ptr [rax - 16]
+0x62,0xf1,0x7c,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovd   -16(%rax), %k0
+# INTEL: {evex} kmovd   k0, dword ptr [rax - 16]
+0x62,0xf1,0xfd,0x08,0x90,0x40,0xf0
+
+# ATT:   {evex} kmovq   -16(%rax), %k0
+# INTEL: {evex} 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}

KanRobert
KanRobert approved these changes Sep 23, 2024
View reviewed changes
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!

@phoebewang phoebewang merged commit 0d334d8 into llvm:main Sep 23, 2024
11 checks passed
@phoebewang phoebewang deleted the APX branch September 23, 2024 01:42
@phoebewang phoebewang added this to the LLVM 19.X Release milestone Sep 23, 2024
@phoebewang
Copy link
Contributor Author

/cherry-pick

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2024

Failed to create pull request for issue109579 https://github.com/llvm/llvm-project/actions/runs/10990045701

@phoebewang
Copy link
Contributor Author

/cherry-pick

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2024

Failed to create pull request for issue109579 https://github.com/llvm/llvm-project/actions/runs/10990148011

phoebewang added a commit to phoebewang/llvm-project that referenced this pull request Sep 23, 2024
@phoebewang
[X86][APX] Fix wrong encoding of promoted KMOV instructions due to mi…
b403d2a 
…ssing NoCD8 (llvm#109579)

Promoted KMOV* was encoded with CD8 incorrectly, see
https://godbolt.org/z/cax513hG1
@phoebewang
Copy link
Contributor Author

Manually cherry-pick in #109635

phoebewang added a commit to phoebewang/llvm-project that referenced this pull request Sep 24, 2024
@phoebewang
[X86][APX] Do not emit {evex} prefix for memory variant
256a55d 
This was mistakely changed by llvm#109579, which doesn't match with other
EVEX decoding.
@phoebewang phoebewang mentioned this pull request Sep 24, 2024
Merged
phoebewang added a commit that referenced this pull request Sep 24, 2024
@phoebewang
[X86][APX] Do not emit {evex} prefix for memory variant (#109759)
70529b2 
This was mistakely changed by #109579, which doesn't match with other
EVEX decoding.
@phoebewang
Copy link
Contributor Author

/cherry-pick 0d334d8 70529b2

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Sep 24, 2024
@phoebewang @llvmbot
[X86][APX] Fix wrong encoding of promoted KMOV instructions due to mi…
0ab42b3 
…ssing NoCD8 (llvm#109579)

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

(cherry picked from commit 0d334d8)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Sep 24, 2024
@phoebewang @llvmbot
[X86][APX] Do not emit {evex} prefix for memory variant (llvm#109759)
9634633 
This was mistakely changed by llvm#109579, which doesn't match with other
EVEX decoding.

(cherry picked from commit 70529b2)
@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

/pull-request #109767

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
@phoebewang @tru
[X86][APX] Fix wrong encoding of promoted KMOV instructions due to mi…
dba2fe7 
…ssing NoCD8 (llvm#109579)

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

(cherry picked from commit 0d334d8)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
@phoebewang @tru
[X86][APX] Do not emit {evex} prefix for memory variant (llvm#109759)
eefadd1 
This was mistakely changed by llvm#109579, which doesn't match with other
EVEX decoding.

(cherry picked from commit 70529b2)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
@phoebewang @tru
[X86][APX] Fix wrong encoding of promoted KMOV instructions due to mi…
edd018e 
…ssing NoCD8 (llvm#109579)

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

(cherry picked from commit 0d334d8)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
@phoebewang @tru
[X86][APX] Do not emit {evex} prefix for memory variant (llvm#109759)
d401987 
This was mistakely changed by llvm#109579, which doesn't match with other
EVEX decoding.

(cherry picked from commit 70529b2)
@thewtex thewtex mentioned this pull request Feb 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KanRobert KanRobert KanRobert approved these changes

@RKSimon RKSimon Awaiting requested review from RKSimon

@FreddyLeaf FreddyLeaf Awaiting requested review from FreddyLeaf

Assignees
No one assigned
Labels
backend:X86 mc Machine (object) code
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 19.X Release
Development

Successfully merging this pull request may close these issues.
3 participants
@phoebewang @llvmbot @KanRobert