From 6e976fbb4b6f38dc06877f25d4b2b47de90a307b Mon Sep 17 00:00:00 2001
From: Gabe Black <gabe.black@gmail.com>
Date: Tue, 2 Feb 2021 19:03:56 -0800
Subject: [PATCH] sim: Get rid of the IsConforming type trait template.

The idea of this template was to distinguish types which should
grow/shrink based on the native size of the ABI in question. Or in other
words, if the ABI was 32 bit, the type should also be 32 bit, or 64 bit
and 64 bit.

Unfortunately, I had intended for Addr to be a conforming type (since
local pointers would be conforming), but uint64_t not to be. Since Addr
is defined as a typedef of uint64_t, the compiler would make *both*
types conforming, giving incorrect behavior on 32 bit systems.

Local pointers will need to be handled in a different way, likely with
the VPtr template, so that they will be treated correctly and not like
an explicitly 64 bit data type.

Change-Id: Idfdd5351260b48bb531a1926b93e0478a297826d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40495
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
---
 src/arch/arm/aapcs32.hh           |  9 +-------
 src/arch/arm/reg_abi.hh           |  1 +
 src/arch/sparc/se_workload.hh     |  1 +
 src/arch/x86/linux/se_workload.hh |  2 +-
 src/sim/syscall_abi.hh            | 36 ++++++-------------------------
 5 files changed, 10 insertions(+), 39 deletions(-)

diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index a0f09b87b..a1345bd57 100644
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -160,9 +160,7 @@ struct Result<Aapcs32, Integer, typename std::enable_if_t<
     static void
     store(ThreadContext *tc, const Integer &i)
     {
-        if (std::is_same<Integer, Addr>::value) {
-            tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
-        } else if (ArmISA::byteOrder(tc) == ByteOrder::little) {
+        if (ArmISA::byteOrder(tc) == ByteOrder::little) {
             tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
             tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
         } else {
@@ -199,11 +197,6 @@ struct Argument<Aapcs32, Integer, typename std::enable_if_t<
     static Integer
     get(ThreadContext *tc, Aapcs32::State &state)
     {
-        if (std::is_same<Integer, Addr>::value &&
-                state.ncrn <= state.MAX_CRN) {
-            return tc->readIntReg(state.ncrn++);
-        }
-
         if (alignof(Integer) == 8 && (state.ncrn % 2))
             state.ncrn++;
 
diff --git a/src/arch/arm/reg_abi.hh b/src/arch/arm/reg_abi.hh
index eb87eff77..94dea18cf 100644
--- a/src/arch/arm/reg_abi.hh
+++ b/src/arch/arm/reg_abi.hh
@@ -55,6 +55,7 @@ template <typename ABI, typename Arg>
 struct Argument<ABI, Arg,
     typename std::enable_if_t<
         std::is_base_of<ArmISA::RegABI32, ABI>::value &&
+        std::is_integral<Arg>::value &&
         ABI::template IsWide<Arg>::value>>
 {
     static Arg
diff --git a/src/arch/sparc/se_workload.hh b/src/arch/sparc/se_workload.hh
index e39261dc5..7303010d8 100644
--- a/src/arch/sparc/se_workload.hh
+++ b/src/arch/sparc/se_workload.hh
@@ -105,6 +105,7 @@ struct Result<ABI, SyscallReturn,
 template <typename Arg>
 struct Argument<SparcISA::SEWorkload::SyscallABI32, Arg,
     typename std::enable_if_t<
+        std::is_integral<Arg>::value &&
         SparcISA::SEWorkload::SyscallABI32::IsWide<Arg>::value>>
 {
     using ABI = SparcISA::SEWorkload::SyscallABI32;
diff --git a/src/arch/x86/linux/se_workload.hh b/src/arch/x86/linux/se_workload.hh
index c1cd2341a..cd26e04f4 100644
--- a/src/arch/x86/linux/se_workload.hh
+++ b/src/arch/x86/linux/se_workload.hh
@@ -93,7 +93,7 @@ namespace GuestABI
 
 template <typename Arg>
 struct Argument<X86ISA::EmuLinux::SyscallABI32, Arg,
-    typename std::enable_if_t<
+    typename std::enable_if_t<std::is_integral<Arg>::value &&
         X86ISA::EmuLinux::SyscallABI32::IsWide<Arg>::value>>
 {
     using ABI = X86ISA::EmuLinux::SyscallABI32;
diff --git a/src/sim/syscall_abi.hh b/src/sim/syscall_abi.hh
index 9d552028f..a60af425f 100644
--- a/src/sim/syscall_abi.hh
+++ b/src/sim/syscall_abi.hh
@@ -36,18 +36,6 @@
 
 class SyscallDesc;
 
-namespace GuestABI
-{
-
-// Does this normally 64 bit data type shrink down to 32 bits for 32 bit ABIs?
-template <typename T, typename Enabled=void>
-struct IsConforming : public std::false_type {};
-
-template <>
-struct IsConforming<Addr> : public std::true_type {};
-
-} // namespace GuestABI
-
 struct GenericSyscallABI
 {
     using State = int;
@@ -64,25 +52,12 @@ struct GenericSyscallABI32 : public GenericSyscallABI
 
     // Is this argument too big for a single register?
     template <typename T, typename Enabled=void>
-    struct IsWide;
+    struct IsWide : public std::false_type {};
 
     template <typename T>
-    struct IsWide<T, typename std::enable_if_t<
-        std::is_integral<T>::value &&
-        (sizeof(T) <= sizeof(UintPtr) ||
-         GuestABI::IsConforming<T>::value)>>
-    {
-        static const bool value = false;
-    };
-
-    template <typename T>
-    struct IsWide<T, typename std::enable_if_t<
-        std::is_integral<T>::value &&
-        (sizeof(T) > sizeof(UintPtr)) &&
-        !GuestABI::IsConforming<T>::value>>
-    {
-        static const bool value = true;
-    };
+    struct IsWide<T, std::enable_if_t<(sizeof(T) > sizeof(UintPtr))>> :
+        public std::true_type
+    {};
 
     // Read two registers and merge them into one value.
     static uint64_t
@@ -117,7 +92,8 @@ struct Argument<ABI, Arg,
 // arguments aren't handled generically.
 template <typename ABI, typename Arg>
 struct Argument<ABI, Arg,
-    typename std::enable_if_t<!ABI::template IsWide<Arg>::value>>
+    typename std::enable_if_t<std::is_integral<Arg>::value &&
+        !ABI::template IsWide<Arg>::value>>
 {
     static Arg
     get(ThreadContext *tc, typename ABI::State &state)
-- 
2.30.2