From 1aa51bf37e6c6672d9e7ae79a7a4854b409c2794 Mon Sep 17 00:00:00 2001
From: Tanner Gooding <tagoo@outlook.com>
Date: Wed, 14 Aug 2024 10:50:55 -0700
Subject: [PATCH] Ensure that we're computing the correct memory operand size
 for disassembly

---
 src/coreclr/jit/emit.h        | 55 +++++------------------------------
 src/coreclr/jit/emitxarch.cpp |  2 +-
 2 files changed, 8 insertions(+), 49 deletions(-)

diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h
index 20a72a96dd780c..d3ae09a06232ee 100644
--- a/src/coreclr/jit/emit.h
+++ b/src/coreclr/jit/emit.h
@@ -2249,8 +2249,7 @@ class emitter
     ssize_t  emitGetInsCIdisp(instrDesc* id);
     unsigned emitGetInsCIargs(instrDesc* id);
 
-    inline emitAttr emitGetMemOpSize(instrDesc* id) const;
-    inline emitAttr emitGetBaseMemOpSize(instrDesc*) const;
+    inline emitAttr emitGetMemOpSize(instrDesc* id, bool ignoreEmbeddedBroadcast = false) const;
 
     // Return the argument count for a direct call "id".
     int emitGetInsCDinfo(instrDesc* id);
@@ -3958,51 +3957,11 @@ inline unsigned emitter::emitGetInsCIargs(instrDesc* id)
 //-----------------------------------------------------------------------------
 // emitGetMemOpSize: Get the memory operand size of instrDesc.
 //
-//  Note: there are cases when embedded broadcast is enabled, so the memory operand
-//  size is different from the intrinsic simd size, we will check here if emitter is
-//  emiting a embedded broadcast enabled instruction.
-
-//  Arguments:
-//       id - Instruction descriptor
-//
-emitAttr emitter::emitGetMemOpSize(instrDesc* id) const
-{
-    if (id->idIsEvexbContextSet())
-    {
-        // should have the assumption that Evex.b now stands for the embedded broadcast context.
-        // reference: Section 2.7.5 in Intel 64 and ia-32 architectures software developer's manual volume 2.
-        ssize_t inputSize = GetInputSizeInBytes(id);
-        switch (inputSize)
-        {
-            case 4:
-                return EA_4BYTE;
-            case 8:
-                return EA_8BYTE;
-
-            default:
-                unreached();
-        }
-    }
-    else
-    {
-        return emitGetBaseMemOpSize(id);
-    }
-}
-
-//-----------------------------------------------------------------------------
-// emitGetMemOpSize: Get the memory operand size of instrDesc.
-//
-// Note: vextractf128 has a 128-bit output (register or memory) but a 256-bit input (register).
-// vinsertf128 is the inverse with a 256-bit output (register), a 256-bit input(register),
-// and a 128-bit input (register or memory).
-// Similarly, vextractf64x4 has a 256-bit output and 128-bit input and vinsertf64x4 the inverse
-// This method is mainly used for such instructions to return the appropriate memory operand
-// size, otherwise returns the regular operand size of the instruction.
-
 //  Arguments:
-//       id - Instruction descriptor
+//    id                      - Instruction descriptor
+//    ignoreEmbeddedBroadcast - true to get the non-embedded operand size; otherwise false
 //
-emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const
+emitAttr emitter::emitGetMemOpSize(instrDesc* id, bool ignoreEmbeddedBroadcast) const
 {
     ssize_t memSize = 0;
 
@@ -4018,7 +3977,7 @@ emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const
     else if (tupleType == INS_TT_FULL)
     {
         // Embedded broadcast supported, so either loading scalar or full vector
-        if (id->idIsEvexbContextSet())
+        if (id->idIsEvexbContextSet() && !ignoreEmbeddedBroadcast)
         {
             memSize = GetInputSizeInBytes(id);
         }
@@ -4040,7 +3999,7 @@ emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const
         {
             memSize = 16;
         }
-        else if (id->idIsEvexbContextSet())
+        else if (id->idIsEvexbContextSet() && !ignoreEmbeddedBroadcast)
         {
             memSize = GetInputSizeInBytes(id);
         }
@@ -4052,7 +4011,7 @@ emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const
     else if (tupleType == INS_TT_HALF)
     {
         // Embedded broadcast supported, so either loading scalar or half vector
-        if (id->idIsEvexbContextSet())
+        if (id->idIsEvexbContextSet() && !ignoreEmbeddedBroadcast)
         {
             memSize = GetInputSizeInBytes(id);
         }
diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp
index 808fa1360337c5..e3561bbff37978 100644
--- a/src/coreclr/jit/emitxarch.cpp
+++ b/src/coreclr/jit/emitxarch.cpp
@@ -10987,7 +10987,7 @@ void emitter::emitDispEmbBroadcastCount(instrDesc* id) const
         return;
     }
     ssize_t baseSize   = GetInputSizeInBytes(id);
-    ssize_t vectorSize = (ssize_t)emitGetBaseMemOpSize(id);
+    ssize_t vectorSize = (ssize_t)emitGetMemOpSize(id, /* ignoreEmbeddedBroadcast */ true);
     printf(" {1to%d}", vectorSize / baseSize);
 }