[llvm] 0b06301 - [X86] Fix shuffle comment decoding for vinsertps immediate operand (#117009)

[via llvm-commits]

Author: Nabeel Omer
Date: 2024-11-21T11:57:31Z
New Revision: 0b06301a1d839eb5f73559f6c3daf9049c34f3af

URL: https://github.com/llvm/llvm-project/commit/0b06301a1d839eb5f73559f6c3daf9049c34f3af
DIFF: https://github.com/llvm/llvm-project/commit/0b06301a1d839eb5f73559f6c3daf9049c34f3af.diff

LOG: [X86] Fix shuffle comment decoding for vinsertps immediate operand (#117009)

The relevant bit from the Intel SDM for vinsertps semantics:
```
IF (SRC = REG) THEN COUNT_S := imm8[7:6] ELSE COUNT_S := 0
```

This is now taken into account.

Added: 
    llvm/test/MC/X86/vinsertps_decode.s

Modified: 
    llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
    llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.cpp
    llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.h
    llvm/lib/Target/X86/X86ISelLowering.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
index 70c71273d270f9..9f8bc57fbc76d4 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
@@ -1122,15 +1122,21 @@ bool llvm::EmitAnyX86InstComments(const MCInst *MI, raw_ostream &OS,
   case X86::VINSERTPSrri:
   case X86::VINSERTPSZrri:
     Src2Name = getRegName(MI->getOperand(2).getReg());
-    [[fallthrough]];
+    DestName = getRegName(MI->getOperand(0).getReg());
+    Src1Name = getRegName(MI->getOperand(1).getReg());
+    if (MI->getOperand(NumOperands - 1).isImm())
+      DecodeINSERTPSMask(MI->getOperand(NumOperands - 1).getImm(), ShuffleMask,
+                         /*SrcIsMem=*/false);
+    break;
+
   case X86::INSERTPSrmi:
   case X86::VINSERTPSrmi:
   case X86::VINSERTPSZrmi:
     DestName = getRegName(MI->getOperand(0).getReg());
     Src1Name = getRegName(MI->getOperand(1).getReg());
     if (MI->getOperand(NumOperands - 1).isImm())
-      DecodeINSERTPSMask(MI->getOperand(NumOperands - 1).getImm(),
-                         ShuffleMask);
+      DecodeINSERTPSMask(MI->getOperand(NumOperands - 1).getImm(), ShuffleMask,
+                         /*SrcIsMem=*/true);
     break;
 
   case X86::MOVLHPSrr:

diff  --git a/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.cpp
index 82f4460a42e70a..933fd16a5cabed 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.cpp
@@ -23,7 +23,8 @@
 
 namespace llvm {
 
-void DecodeINSERTPSMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
+void DecodeINSERTPSMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask,
+                        bool SrcIsMem) {
   // Defaults the copying the dest value.
   ShuffleMask.push_back(0);
   ShuffleMask.push_back(1);
@@ -33,7 +34,7 @@ void DecodeINSERTPSMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask) {
   // Decode the immediate.
   unsigned ZMask = Imm & 15;
   unsigned CountD = (Imm >> 4) & 3;
-  unsigned CountS = (Imm >> 6) & 3;
+  unsigned CountS = SrcIsMem ? 0 : (Imm >> 6) & 3;
 
   // CountS selects which input element to use.
   unsigned InVal = 4 + CountS;

diff  --git a/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.h b/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.h
index 4ef9959f7a278c..b58e3a73a8d562 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.h
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86ShuffleDecode.h
@@ -28,7 +28,8 @@ template <typename T> class SmallVectorImpl;
 enum { SM_SentinelUndef = -1, SM_SentinelZero = -2 };
 
 /// Decode a 128-bit INSERTPS instruction as a v4f32 shuffle mask.
-void DecodeINSERTPSMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask);
+void DecodeINSERTPSMask(unsigned Imm, SmallVectorImpl<int> &ShuffleMask,
+                        bool SrcIsMem);
 
 // Insert the bottom Len elements from a second source into a vector starting at
 // element Idx.

diff  --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 2b497f36fc6e06..934654a09c1724 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -5372,7 +5372,7 @@ static bool getTargetShuffleMask(SDValue N, bool AllowSentinelZero,
     assert(N.getOperand(0).getValueType() == VT && "Unexpected value type");
     assert(N.getOperand(1).getValueType() == VT && "Unexpected value type");
     ImmN = N.getConstantOperandVal(N.getNumOperands() - 1);
-    DecodeINSERTPSMask(ImmN, Mask);
+    DecodeINSERTPSMask(ImmN, Mask, /*SrcIsMem=*/false);
     IsUnary = IsFakeUnary = N.getOperand(0) == N.getOperand(1);
     break;
   case X86ISD::EXTRQI:

diff  --git a/llvm/test/MC/X86/vinsertps_decode.s b/llvm/test/MC/X86/vinsertps_decode.s
new file mode 100644
index 00000000000000..b200fb14aefffe
--- /dev/null
+++ b/llvm/test/MC/X86/vinsertps_decode.s
@@ -0,0 +1,11 @@
+# RUN: llvm-mc -triple x86_64-unknown-unknown %s | FileCheck %s
+
+.intel_syntax
+
+# CHECK: insertps  $176, (%rax), %xmm2        # xmm2 = xmm2[0,1,2],mem[0]
+# CHECK: vinsertps $176, (%rax), %xmm2, %xmm2 # xmm2 = xmm2[0,1,2],mem[0]
+# CHECK: vinsertps $176, (%rax), %xmm29, %xmm0 # xmm0 = xmm29[0,1,2],mem[0]
+
+insertps xmm2, dword ptr [rax], 0x0B0
+vinsertps xmm2,xmm2,dword ptr [rax],0x0B0
+vinsertps xmm0,xmm29,dword ptr [rax],0x0B0