x86: Rework how "split" loads/stores are handled.

Explicitly separate the way the data is represented in the underlying
representation from how it's represented in the instruction.

In order to make the ISA parser happy, the Mem operand needs to have
a single, particular type. To handle that with scalar types, we just
used uint64_ts and then worked with values that were smaller than the
maximum we could hold. To work with these new array values, we also
use an underlying uint64_t for each element.

To make accessing the underlying memory system more natural, when we
go to actually read or write values, we translate the access into an
array of the actual, correct underlying type. That way we don't have
non-exact asserts which confuse gcc, or weird endianness conversion
which assumes that the data should be flipped 8 bytes at a time.

Because the functions involved are generally inline, the syntactic
niceness should all boil off, and the final implementation in the
binary should be simple and efficient for the given data types.

Change-Id: I14ce7a2fe0dc2cbaf6ad4a0d19f743c45ee78e26
Reviewed-on: https://gem5-review.googlesource.com/6582
Reviewed-by: Gabe Black <gabeblack@google.com>
Maintainer: Gabe Black <gabeblack@google.com>

2 files changed, 103 insertions, 94 deletions

diff --git a/src/arch/x86/isa/microops/ldstop.isa b/src/arch/x86/isa/microops/ldstop.isa
index 5ff4f0cea..a3d9c5a70 100644
--- a/src/arch/x86/isa/microops/ldstop.isa
+++ b/src/arch/x86/isa/microops/ldstop.isa
@@ -99,8 +99,7 @@ def template MicroLoadExecute {{
         %(ea_code)s;
         DPRINTF(X86, "%s : %s: The address is %#x\n", instMnem, mnemonic, EA);
 
-        fault = readMemAtomic(xc, traceData, EA, Mem,
-                              %(memDataSize)s, memFlags);
+        fault = readMemAtomic(xc, traceData, EA, Mem, dataSize, memFlags);
 
         if (fault == NoFault) {
             %(code)s;
@@ -145,7 +144,7 @@ def template MicroLoadCompleteAcc {{
         %(op_decl)s;
         %(op_rd)s;
 
-        getMem(pkt, Mem, %(memDataSize)s, traceData);
+        getMem(pkt, Mem, dataSize, traceData);
 
         %(code)s;
 
@@ -174,12 +173,10 @@ def template MicroStoreExecute {{
 
         %(code)s;
 
-        if(fault == NoFault)
-        {
-            fault = writeMemAtomic(xc, traceData, Mem, %(memDataSize)s, EA,
-                    memFlags, NULL);
-            if(fault == NoFault)
-            {
+        if (fault == NoFault) {
+            fault = writeMemAtomic(xc, traceData, Mem, dataSize, EA,
+                                   memFlags, NULL);
+            if (fault == NoFault) {
                 %(op_wb)s;
             }
         }
@@ -202,10 +199,9 @@ def template MicroStoreInitiateAcc {{
 
         %(code)s;
 
-        if(fault == NoFault)
-        {
-            fault = writeMemTiming(xc, traceData, Mem, %(memDataSize)s, EA,
-                    memFlags, NULL);
+        if (fault == NoFault) {
+            fault = writeMemTiming(xc, traceData, Mem, dataSize, EA,
+                                   memFlags, NULL);
         }
         return fault;
     }
@@ -561,18 +557,9 @@ let {{
         microopClasses[name] = LoadOp
 
     code = '''
-        switch (dataSize) {
-          case 4:
-            DataLow = bits(Mem_u2qw[0], 31, 0);
-            DataHi  = bits(Mem_u2qw[0], 63, 32);
-            break;
-          case 8:
-            DataLow = Mem_u2qw[0];
-            DataHi  = Mem_u2qw[1];
-            break;
-          default:
-            panic("Unhandled data size %d in LdSplit.\\n", dataSize);
-        }'''
+        DataLow = Mem_u2qw[0];
+        DataHi = Mem_u2qw[1];
+    '''
 
     defineMicroLoadSplitOp('LdSplit', code,
                            '(StoreCheck << FlagShift)')
@@ -683,17 +670,9 @@ let {{
         microopClasses[name] = StoreOp
 
     code = '''
-        switch (dataSize) {
-          case 4:
-            Mem_u2qw[0] = (DataHi << 32) | DataLow;
-            break;
-          case 8:
-            Mem_u2qw[0] = DataLow;
-            Mem_u2qw[1] = DataHi;
-            break;
-          default:
-            panic("Unhandled data size %d in StSplit.\\n", dataSize);
-        }'''
+        Mem_u2qw[0] = DataLow;
+        Mem_u2qw[1] = DataHi;
+    '''
 
     defineMicroStoreSplitOp('StSplit', code);
 
diff --git a/src/arch/x86/memhelpers.hh b/src/arch/x86/memhelpers.hh
index 00a6e9a33..aa3617b43 100644
--- a/src/arch/x86/memhelpers.hh
+++ b/src/arch/x86/memhelpers.hh
@@ -74,24 +74,30 @@ getMem(PacketPtr pkt, uint64_t &mem, unsigned dataSize,
         traceData->setData(mem);
 }
 
+template <typename T, size_t N>
+static void
+getPackedMem(PacketPtr pkt, std::array<uint64_t, N> &mem, unsigned dataSize)
+{
+    std::array<T, N> real_mem = pkt->get<std::array<T, N> >();
+    for (int i = 0; i < N; i++)
+        mem[i] = real_mem[i];
+}
 
 template <size_t N>
-void
+static void
 getMem(PacketPtr pkt, std::array<uint64_t, N> &mem, unsigned dataSize,
        Trace::InstRecord *traceData)
 {
-    assert(dataSize >= 8);
-    assert((dataSize % 8) == 0);
-
-    int num_words = dataSize / 8;
-    assert(num_words <= N);
-
-    auto pkt_data = pkt->getConstPtr<const uint64_t>();
-    for (int i = 0; i < num_words; ++i)
-        mem[i] = gtoh(pkt_data[i]);
-
-    // traceData record only has space for 64 bits, so we just record
-    // the first qword
+    switch (dataSize) {
+      case 4:
+        getPackedMem<uint32_t, N>(pkt, mem, dataSize);
+        break;
+      case 8:
+        getPackedMem<uint64_t, N>(pkt, mem, dataSize);
+        break;
+      default:
+        panic("Unhandled element size in getMem.\n");
+    }
     if (traceData)
         traceData->setData(mem[0]);
 }
@@ -114,62 +120,86 @@ readMemAtomic(ExecContext *xc, Trace::InstRecord *traceData, Addr addr,
     return fault;
 }
 
+template <typename T, size_t N>
+static Fault
+readPackedMemAtomic(ExecContext *xc, Addr addr, std::array<uint64_t, N> &mem,
+                    unsigned flags)
+{
+    std::array<T, N> real_mem;
+    Fault fault = xc->readMem(addr, (uint8_t *)&real_mem,
+                              sizeof(T) * N, flags);
+    if (fault == NoFault) {
+        real_mem = gtoh(real_mem);
+        for (int i = 0; i < N; i++)
+            mem[i] = real_mem[i];
+    }
+    return fault;
+}
+
 template <size_t N>
-Fault
+static Fault
 readMemAtomic(ExecContext *xc, Trace::InstRecord *traceData, Addr addr,
               std::array<uint64_t, N> &mem, unsigned dataSize,
               unsigned flags)
 {
-    assert(dataSize >= 8);
-    assert((dataSize % 8) == 0);
-
-    Fault fault = xc->readMem(addr, (uint8_t *)&mem, dataSize, flags);
-
-    if (fault == NoFault) {
-        int num_words = dataSize / 8;
-        assert(num_words <= N);
-
-        for (int i = 0; i < num_words; ++i)
-            mem[i] = gtoh(mem[i]);
+    Fault fault = NoFault;
 
-        if (traceData)
-            traceData->setData(mem[0]);
+    switch (dataSize) {
+      case 4:
+        fault = readPackedMemAtomic<uint32_t, N>(xc, addr, mem, flags);
+        break;
+      case 8:
+        fault = readPackedMemAtomic<uint64_t, N>(xc, addr, mem, flags);
+        break;
+      default:
+        panic("Unhandled element size in readMemAtomic\n");
     }
+    if (fault == NoFault && traceData)
+        traceData->setData(mem[0]);
     return fault;
 }
 
+template <typename T, size_t N>
+static Fault
+writePackedMem(ExecContext *xc, std::array<uint64_t, N> &mem, Addr addr,
+               unsigned flags, uint64_t *res)
+{
+    std::array<T, N> real_mem;
+    for (int i = 0; i < N; i++)
+        real_mem[i] = mem[i];
+    real_mem = htog(real_mem);
+    return xc->writeMem((uint8_t *)&real_mem, sizeof(T) * N,
+                        addr, flags, res);
+}
+
 static Fault
 writeMemTiming(ExecContext *xc, Trace::InstRecord *traceData, uint64_t mem,
                unsigned dataSize, Addr addr, Request::Flags flags,
                uint64_t *res)
 {
-    if (traceData) {
+    if (traceData)
         traceData->setData(mem);
-    }
     mem = TheISA::htog(mem);
     return xc->writeMem((uint8_t *)&mem, dataSize, addr, flags, res);
 }
 
 template <size_t N>
-Fault
+static Fault
 writeMemTiming(ExecContext *xc, Trace::InstRecord *traceData,
                std::array<uint64_t, N> &mem, unsigned dataSize,
                Addr addr, unsigned flags, uint64_t *res)
 {
-    assert(dataSize >= 8);
-    assert((dataSize % 8) == 0);
-
-    if (traceData) {
+    if (traceData)
         traceData->setData(mem[0]);
-    }
-
-    int num_words = dataSize / 8;
-    assert(num_words <= N);
 
-    for (int i = 0; i < num_words; ++i)
-        mem[i] = htog(mem[i]);
-
-    return xc->writeMem((uint8_t *)&mem, dataSize, addr, flags, res);
+    switch (dataSize) {
+      case 4:
+        return writePackedMem<uint32_t, N>(xc, mem, addr, flags, res);
+      case 8:
+        return writePackedMem<uint64_t, N>(xc, mem, addr, flags, res);
+      default:
+        panic("Unhandled element size in writeMemTiming.\n");
+    }
 }
 
 static Fault
@@ -177,39 +207,39 @@ writeMemAtomic(ExecContext *xc, Trace::InstRecord *traceData, uint64_t mem,
                unsigned dataSize, Addr addr, Request::Flags flags,
                uint64_t *res)
 {
-    if (traceData) {
+    if (traceData)
         traceData->setData(mem);
-    }
     uint64_t host_mem = TheISA::htog(mem);
     Fault fault =
           xc->writeMem((uint8_t *)&host_mem, dataSize, addr, flags, res);
-    if (fault == NoFault && res != NULL) {
+    if (fault == NoFault && res)
         *res = gtoh(*res);
-    }
     return fault;
 }
 
 template <size_t N>
-Fault
+static Fault
 writeMemAtomic(ExecContext *xc, Trace::InstRecord *traceData,
                std::array<uint64_t, N> &mem, unsigned dataSize,
                Addr addr, unsigned flags, uint64_t *res)
 {
-    if (traceData) {
+    if (traceData)
         traceData->setData(mem[0]);
-    }
-
-    int num_words = dataSize / 8;
-    assert(num_words <= N);
-
-    for (int i = 0; i < num_words; ++i)
-        mem[i] = htog(mem[i]);
 
-    Fault fault = xc->writeMem((uint8_t *)&mem, dataSize, addr, flags, res);
+    Fault fault;
+    switch (dataSize) {
+      case 4:
+        fault = writePackedMem<uint32_t, N>(xc, mem, addr, flags, res);
+        break;
+      case 8:
+        fault = writePackedMem<uint64_t, N>(xc, mem, addr, flags, res);
+        break;
+      default:
+        panic("Unhandled element size in writeMemAtomic.\n");
+    }
 
-    if (fault == NoFault && res != NULL) {
+    if (fault == NoFault && res)
         *res = gtoh(*res);
-    }
 
     return fault;
 }