Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/40495 )

Change subject: sim: Get rid of the IsConforming type trait template.
......................................................................

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.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/arm/aapcs32.hh
M src/arch/arm/reg_abi.hh
M src/arch/sparc/se_workload.hh
M src/arch/x86/linux/se_workload.hh
M src/sim/syscall_abi.hh
5 files changed, 10 insertions(+), 39 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index a0f09b8..a1345bd 100644
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -160,9 +160,7 @@
     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 @@
     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 eb87eff..94dea18 100644
--- a/src/arch/arm/reg_abi.hh
+++ b/src/arch/arm/reg_abi.hh
@@ -55,6 +55,7 @@
 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 e39261d..7303010 100644
--- a/src/arch/sparc/se_workload.hh
+++ b/src/arch/sparc/se_workload.hh
@@ -105,6 +105,7 @@
 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 c1cd234..cd26e04 100644
--- a/src/arch/x86/linux/se_workload.hh
+++ b/src/arch/x86/linux/se_workload.hh
@@ -93,7 +93,7 @@

 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 9d55202..a60af42 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 @@

     // 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 @@
 // 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)



The change was submitted with unreviewed changes in the following files:

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40495
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Idfdd5351260b48bb531a1926b93e0478a297826d
Gerrit-Change-Number: 40495
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Jonathan Bohren <jonathan.boh...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to