Skip to content

[X86][MC] Fix wrong encoding of promoted BMI instructions due to missing NoCD8 #78386

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 6 commits into from
Jan 18, 2024
Merged

[X86][MC] Fix wrong encoding of promoted BMI instructions due to missing NoCD8 #78386

merged 6 commits into from
Jan 18, 2024

Conversation

XinWang10
Copy link
Contributor

@XinWang10 XinWang10 commented Jan 17, 2024 •
edited by KanRobert
Loading

Address review comments in #76709

Add NoCD8 to class ITy, and rewrite the promoted instructions with ITy to avoid unexpected incorrect encoding about NoCD8.

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

llvmbot commented Jan 17, 2024 •
edited
Loading

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-mc

Author: None (XinWang10)

Changes

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

5 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrSSE.td (+2-2)
  • (modified) llvm/lib/Target/X86/X86InstrUtils.td (+3-3)
  • (modified) llvm/test/MC/Disassembler/X86/apx/bmi2.txt (+10-10)
  • (modified) llvm/test/MC/X86/apx/bmi2-att.s (+10-10)
  • (modified) llvm/test/MC/X86/apx/bmi2-intel.s (+10-10)
diff --git a/llvm/lib/Target/X86/X86InstrSSE.td b/llvm/lib/Target/X86/X86InstrSSE.td
index a8cd1996eeb356b..7d94fec9a354d04 100644
--- a/llvm/lib/Target/X86/X86InstrSSE.td
+++ b/llvm/lib/Target/X86/X86InstrSSE.td
@@ -6663,14 +6663,14 @@ let Defs = [ECX, EFLAGS], Uses = [EAX, EDX], hasSideEffects = 0 in {
 class Crc32r<X86TypeInfo t, RegisterClass rc, SDPatternOperator node>
   : ITy<0xF1, MRMSrcReg, t, (outs rc:$dst), (ins rc:$src1, t.RegClass:$src2),
       "crc32", binop_args, [(set rc:$dst, (node rc:$src1, t.RegClass:$src2))]>,
-    Sched<[WriteCRC32]>, NoCD8 {
+    Sched<[WriteCRC32]> {
   let Constraints = "$src1 = $dst";
 }
 
 class Crc32m<X86TypeInfo t, RegisterClass rc, SDPatternOperator node>
   : ITy<0xF1, MRMSrcMem, t, (outs rc:$dst), (ins rc:$src1, t.MemOperand:$src2),
       "crc32", binop_args, [(set rc:$dst, (node rc:$src1, (load addr:$src2)))]>,
-    Sched<[WriteCRC32.Folded, WriteCRC32.ReadAfterFold]>, NoCD8 {
+    Sched<[WriteCRC32.Folded, WriteCRC32.ReadAfterFold]> {
   let Constraints = "$src1 = $dst";
 }
 
diff --git a/llvm/lib/Target/X86/X86InstrUtils.td b/llvm/lib/Target/X86/X86InstrUtils.td
index e79240ae443ade5..27aeff1cd3ae2c0 100644
--- a/llvm/lib/Target/X86/X86InstrUtils.td
+++ b/llvm/lib/Target/X86/X86InstrUtils.td
@@ -113,9 +113,9 @@ class NDD<bit ndd, Map map = OB> {
   Map OpMap = !if(!eq(ndd, 0), map, T_MAP4);
 }
 // NF - Helper for NF (no flags update) instructions
-class NF: T_MAP4, EVEX, EVEX_NF, NoCD8;
+class NF: T_MAP4, EVEX, EVEX_NF;
 // PL - Helper for promoted legacy instructions
-class PL: T_MAP4, EVEX, NoCD8, ExplicitEVEXPrefix;
+class PL: T_MAP4, EVEX, ExplicitEVEXPrefix;
 
 //===----------------------------------------------------------------------===//
 // X86 Type infomation definitions
@@ -961,7 +961,7 @@ class ITy<bits<8> o, Format f, X86TypeInfo t, dag outs, dag ins, string m,
           string args, list<dag> p>
   : I<{o{7}, o{6}, o{5}, o{4}, o{3}, o{2}, o{1},
        !if(!eq(t.HasEvenOpcode, 1), 0, o{0})}, f, outs, ins,
-      !strconcat(m, "{", t.InstrSuffix, "}\t", args), p> {
+      !strconcat(m, "{", t.InstrSuffix, "}\t", args), p>, NoCD8 {
   let hasSideEffects = 0;
   let hasREX_W  = t.HasREX_W;
 }
diff --git a/llvm/test/MC/Disassembler/X86/apx/bmi2.txt b/llvm/test/MC/Disassembler/X86/apx/bmi2.txt
index 0fb11f4061f1b93..428cf2d148e2dd1 100644
--- a/llvm/test/MC/Disassembler/X86/apx/bmi2.txt
+++ b/llvm/test/MC/Disassembler/X86/apx/bmi2.txt
@@ -13,11 +13,11 @@
 
 # ATT:   mulxl	123(%rax,%rbx,4), %ecx, %edx
 # INTEL: mulx	edx, ecx, dword ptr [rax + 4*rbx + 123]
-0x62,0xf2,0x77,0x08,0xf6,0x94,0x98,0x7b,0x00,0x00,0x00
+0x62,0xf2,0x77,0x08,0xf6,0x54,0x98,0x7b
 
 # ATT:   mulxq	123(%rax,%rbx,4), %r9, %r15
 # INTEL: mulx	r15, r9, qword ptr [rax + 4*rbx + 123]
-0x62,0x72,0xb7,0x08,0xf6,0xbc,0x98,0x7b,0x00,0x00,0x00
+0x62,0x72,0xb7,0x08,0xf6,0x7c,0x98,0x7b
 
 # ATT:   mulxl	%r18d, %r22d, %r26d
 # INTEL: mulx	r26d, r22d, r18d
@@ -115,11 +115,11 @@
 
 # ATT:   rorxl	$123, 123(%rax,%rbx,4), %ecx
 # INTEL: rorx	ecx, dword ptr [rax + 4*rbx + 123], 123
-0x62,0xf3,0x7f,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b
+0x62,0xf3,0x7f,0x08,0xf0,0x4c,0x98,0x7b,0x7b
 
 # ATT:   rorxq	$123, 123(%rax,%rbx,4), %r9
 # INTEL: rorx	r9, qword ptr [rax + 4*rbx + 123], 123
-0x62,0x73,0xff,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b
+0x62,0x73,0xff,0x08,0xf0,0x4c,0x98,0x7b,0x7b
 
 # ATT:   rorxl	$123, %r18d, %r22d
 # INTEL: rorx	r22d, r18d, 123
@@ -145,7 +145,7 @@
 
 # ATT:   sarxl	%ecx, 123(%rax,%rbx,4), %edx
 # INTEL: sarx	edx, dword ptr [rax + 4*rbx + 123], ecx
-0x62,0xf2,0x76,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00
+0x62,0xf2,0x76,0x08,0xf7,0x54,0x98,0x7b
 
 # ATT:   sarxq	%r9, %r15, %r11
 # INTEL: sarx	r11, r15, r9
@@ -153,7 +153,7 @@
 
 # ATT:   sarxq	%r9, 123(%rax,%rbx,4), %r15
 # INTEL: sarx	r15, qword ptr [rax + 4*rbx + 123], r9
-0x62,0x72,0xb6,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00
+0x62,0x72,0xb6,0x08,0xf7,0x7c,0x98,0x7b
 
 # ATT:   sarxl	%r18d, %r22d, %r26d
 # INTEL: sarx	r26d, r22d, r18d
@@ -179,7 +179,7 @@
 
 # ATT:   shlxl	%ecx, 123(%rax,%rbx,4), %edx
 # INTEL: shlx	edx, dword ptr [rax + 4*rbx + 123], ecx
-0x62,0xf2,0x75,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00
+0x62,0xf2,0x75,0x08,0xf7,0x54,0x98,0x7b
 
 # ATT:   shlxq	%r9, %r15, %r11
 # INTEL: shlx	r11, r15, r9
@@ -187,7 +187,7 @@
 
 # ATT:   shlxq	%r9, 123(%rax,%rbx,4), %r15
 # INTEL: shlx	r15, qword ptr [rax + 4*rbx + 123], r9
-0x62,0x72,0xb5,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00
+0x62,0x72,0xb5,0x08,0xf7,0x7c,0x98,0x7b
 
 # ATT:   shlxl	%r18d, %r22d, %r26d
 # INTEL: shlx	r26d, r22d, r18d
@@ -213,7 +213,7 @@
 
 # ATT:   shrxl	%ecx, 123(%rax,%rbx,4), %edx
 # INTEL: shrx	edx, dword ptr [rax + 4*rbx + 123], ecx
-0x62,0xf2,0x77,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00
+0x62,0xf2,0x77,0x08,0xf7,0x54,0x98,0x7b
 
 # ATT:   shrxq	%r9, %r15, %r11
 # INTEL: shrx	r11, r15, r9
@@ -221,7 +221,7 @@
 
 # ATT:   shrxq	%r9, 123(%rax,%rbx,4), %r15
 # INTEL: shrx	r15, qword ptr [rax + 4*rbx + 123], r9
-0x62,0x72,0xb7,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00
+0x62,0x72,0xb7,0x08,0xf7,0x7c,0x98,0x7b
 
 # ATT:   shrxl	%r18d, %r22d, %r26d
 # INTEL: shrx	r26d, r22d, r18d
diff --git a/llvm/test/MC/X86/apx/bmi2-att.s b/llvm/test/MC/X86/apx/bmi2-att.s
index 14e8566e799d59b..20544d7922cfee7 100644
--- a/llvm/test/MC/X86/apx/bmi2-att.s
+++ b/llvm/test/MC/X86/apx/bmi2-att.s
@@ -15,11 +15,11 @@
          {evex}	mulxq	%r9, %r15, %r11
 
 # CHECK: {evex}	mulxl	123(%rax,%rbx,4), %ecx, %edx
-# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf6,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf6,0x54,0x98,0x7b]
          {evex}	mulxl	123(%rax,%rbx,4), %ecx, %edx
 
 # CHECK: {evex}	mulxq	123(%rax,%rbx,4), %r9, %r15
-# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf6,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf6,0x7c,0x98,0x7b]
          {evex}	mulxq	123(%rax,%rbx,4), %r9, %r15
 
 # CHECK: mulxl	%r18d, %r22d, %r26d
@@ -117,11 +117,11 @@
          {evex}	rorxq	$123, %r9, %r15
 
 # CHECK: {evex}	rorxl	$123, 123(%rax,%rbx,4), %ecx
-# CHECK: encoding: [0x62,0xf3,0x7f,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b]
+# CHECK: encoding: [0x62,0xf3,0x7f,0x08,0xf0,0x4c,0x98,0x7b,0x7b]
          {evex}	rorxl	$123, 123(%rax,%rbx,4), %ecx
 
 # CHECK: {evex}	rorxq	$123, 123(%rax,%rbx,4), %r9
-# CHECK: encoding: [0x62,0x73,0xff,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b]
+# CHECK: encoding: [0x62,0x73,0xff,0x08,0xf0,0x4c,0x98,0x7b,0x7b]
          {evex}	rorxq	$123, 123(%rax,%rbx,4), %r9
 
 # CHECK: rorxl	$123, %r18d, %r22d
@@ -147,7 +147,7 @@
          {evex}	sarxl	%ecx, %edx, %r10d
 
 # CHECK: {evex}	sarxl	%ecx, 123(%rax,%rbx,4), %edx
-# CHECK: encoding: [0x62,0xf2,0x76,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x76,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	sarxl	%ecx, 123(%rax,%rbx,4), %edx
 
 # CHECK: {evex}	sarxq	%r9, %r15, %r11
@@ -155,7 +155,7 @@
          {evex}	sarxq	%r9, %r15, %r11
 
 # CHECK: {evex}	sarxq	%r9, 123(%rax,%rbx,4), %r15
-# CHECK: encoding: [0x62,0x72,0xb6,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb6,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	sarxq	%r9, 123(%rax,%rbx,4), %r15
 
 # CHECK: sarxl	%r18d, %r22d, %r26d
@@ -181,7 +181,7 @@
          {evex}	shlxl	%ecx, %edx, %r10d
 
 # CHECK: {evex}	shlxl	%ecx, 123(%rax,%rbx,4), %edx
-# CHECK: encoding: [0x62,0xf2,0x75,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x75,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	shlxl	%ecx, 123(%rax,%rbx,4), %edx
 
 # CHECK: {evex}	shlxq	%r9, %r15, %r11
@@ -189,7 +189,7 @@
          {evex}	shlxq	%r9, %r15, %r11
 
 # CHECK: {evex}	shlxq	%r9, 123(%rax,%rbx,4), %r15
-# CHECK: encoding: [0x62,0x72,0xb5,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb5,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	shlxq	%r9, 123(%rax,%rbx,4), %r15
 
 # CHECK: shlxl	%r18d, %r22d, %r26d
@@ -215,7 +215,7 @@
          {evex}	shrxl	%ecx, %edx, %r10d
 
 # CHECK: {evex}	shrxl	%ecx, 123(%rax,%rbx,4), %edx
-# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	shrxl	%ecx, 123(%rax,%rbx,4), %edx
 
 # CHECK: {evex}	shrxq	%r9, %r15, %r11
@@ -223,7 +223,7 @@
          {evex}	shrxq	%r9, %r15, %r11
 
 # CHECK: {evex}	shrxq	%r9, 123(%rax,%rbx,4), %r15
-# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	shrxq	%r9, 123(%rax,%rbx,4), %r15
 
 # CHECK: shrxl	%r18d, %r22d, %r26d
diff --git a/llvm/test/MC/X86/apx/bmi2-intel.s b/llvm/test/MC/X86/apx/bmi2-intel.s
index f21004fdd696abf..fe96fbc0be8d7a5 100644
--- a/llvm/test/MC/X86/apx/bmi2-intel.s
+++ b/llvm/test/MC/X86/apx/bmi2-intel.s
@@ -11,11 +11,11 @@
          {evex}	mulx	r11, r15, r9
 
 # CHECK: {evex}	mulx	edx, ecx, dword ptr [rax + 4*rbx + 123]
-# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf6,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf6,0x54,0x98,0x7b]
          {evex}	mulx	edx, ecx, dword ptr [rax + 4*rbx + 123]
 
 # CHECK: {evex}	mulx	r15, r9, qword ptr [rax + 4*rbx + 123]
-# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf6,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf6,0x7c,0x98,0x7b]
          {evex}	mulx	r15, r9, qword ptr [rax + 4*rbx + 123]
 
 # CHECK: mulx	r26d, r22d, r18d
@@ -113,11 +113,11 @@
          {evex}	rorx	r15, r9, 123
 
 # CHECK: {evex}	rorx	ecx, dword ptr [rax + 4*rbx + 123], 123
-# CHECK: encoding: [0x62,0xf3,0x7f,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b]
+# CHECK: encoding: [0x62,0xf3,0x7f,0x08,0xf0,0x4c,0x98,0x7b,0x7b]
          {evex}	rorx	ecx, dword ptr [rax + 4*rbx + 123], 123
 
 # CHECK: {evex}	rorx	r9, qword ptr [rax + 4*rbx + 123], 123
-# CHECK: encoding: [0x62,0x73,0xff,0x08,0xf0,0x8c,0x98,0x7b,0x00,0x00,0x00,0x7b]
+# CHECK: encoding: [0x62,0x73,0xff,0x08,0xf0,0x4c,0x98,0x7b,0x7b]
          {evex}	rorx	r9, qword ptr [rax + 4*rbx + 123], 123
 
 # CHECK: rorx	r22d, r18d, 123
@@ -143,7 +143,7 @@
          {evex}	sarx	r10d, edx, ecx
 
 # CHECK: {evex}	sarx	edx, dword ptr [rax + 4*rbx + 123], ecx
-# CHECK: encoding: [0x62,0xf2,0x76,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x76,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	sarx	edx, dword ptr [rax + 4*rbx + 123], ecx
 
 # CHECK: {evex}	sarx	r11, r15, r9
@@ -151,7 +151,7 @@
          {evex}	sarx	r11, r15, r9
 
 # CHECK: {evex}	sarx	r15, qword ptr [rax + 4*rbx + 123], r9
-# CHECK: encoding: [0x62,0x72,0xb6,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb6,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	sarx	r15, qword ptr [rax + 4*rbx + 123], r9
 
 # CHECK: sarx	r26d, r22d, r18d
@@ -177,7 +177,7 @@
          {evex}	shlx	r10d, edx, ecx
 
 # CHECK: {evex}	shlx	edx, dword ptr [rax + 4*rbx + 123], ecx
-# CHECK: encoding: [0x62,0xf2,0x75,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x75,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	shlx	edx, dword ptr [rax + 4*rbx + 123], ecx
 
 # CHECK: {evex}	shlx	r11, r15, r9
@@ -185,7 +185,7 @@
          {evex}	shlx	r11, r15, r9
 
 # CHECK: {evex}	shlx	r15, qword ptr [rax + 4*rbx + 123], r9
-# CHECK: encoding: [0x62,0x72,0xb5,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb5,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	shlx	r15, qword ptr [rax + 4*rbx + 123], r9
 
 # CHECK: shlx	r26d, r22d, r18d
@@ -211,7 +211,7 @@
          {evex}	shrx	r10d, edx, ecx
 
 # CHECK: {evex}	shrx	edx, dword ptr [rax + 4*rbx + 123], ecx
-# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf7,0x94,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0xf2,0x77,0x08,0xf7,0x54,0x98,0x7b]
          {evex}	shrx	edx, dword ptr [rax + 4*rbx + 123], ecx
 
 # CHECK: {evex}	shrx	r11, r15, r9
@@ -219,7 +219,7 @@
          {evex}	shrx	r11, r15, r9
 
 # CHECK: {evex}	shrx	r15, qword ptr [rax + 4*rbx + 123], r9
-# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf7,0xbc,0x98,0x7b,0x00,0x00,0x00]
+# CHECK: encoding: [0x62,0x72,0xb7,0x08,0xf7,0x7c,0x98,0x7b]
          {evex}	shrx	r15, qword ptr [rax + 4*rbx + 123], r9
 
 # CHECK: shrx	r26d, r22d, r18d

@topperc
Copy link
Collaborator

topperc commented Jan 17, 2024

It's good idea to mention what instruction has the wrong encoding in the patch title or description.

@XinWang10 XinWang10 changed the title [X86][MC] Fix wrong encoding due to missing NoCD8 [X86][MC] Fix wrong encoding of promoted BMI instructions due to missing NoCD8 Jan 17, 2024
@KanRobert KanRobert requested a review from topperc January 17, 2024 04:18
@KanRobert
Copy link
Contributor

@XinWang10 I believe you will rewrite other promoted instructions with ITy, right?

@XinWang10
Copy link
Contributor Author

@XinWang10 I believe you will rewrite other promoted instructions with ITy, right?
I just find this existing error now, if others needed to be rewrited, would do it in another NFC patch.

KanRobert
KanRobert approved these changes Jan 17, 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

KanRobert
KanRobert approved these changes Jan 18, 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

@KanRobert KanRobert merged commit d124b02 into llvm:main Jan 18, 2024
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

@phoebewang phoebewang Awaiting requested review from phoebewang

@topperc topperc Awaiting requested review from topperc

Assignees
No one assigned
Labels
backend:X86 mc Machine (object) code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@XinWang10 @llvmbot @topperc @KanRobert