[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix aapcs32/aapcs64 compilation issues
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/26990 )
Change subject: arch-arm: Fix aapcs32/aapcs64 compilation issues
..
arch-arm: Fix aapcs32/aapcs64 compilation issues
Some compilers won't build ARM due to how guest ABI
has been implemented.
The error is: "left shift count >= width of type"
[-Werror=shift-count-overflow]
The error is triggered when there is a left shift > the variable size
(in bits); this leads to undefined behaviour.
This is a compile time vs run time problem; the code is technically
fine, but the compiler is not able to understand this.
For example in aapcs64:
struct Argument::value>::type> : public Aapcs64ArgumentBase
{
[...]
if (sizeof(Integer) == 16 && state.ngrn + 1 <= state.MAX_GRN) {
Integer low = tc->readIntReg(state.ngrn++);
Integer high = tc->readIntReg(state.ngrn++);
high = high << 64;
return high | low;
}
}
Even if the sizeof operator will be evaluated at compile time,
the block will be executed at runtime: the block will still be part of
the code if Integer = uint32_t.
The compiler will then throw an error because we are left shifting an
uint32_t by 64 bits.
Error arising on:
Compiler: gcc/5.4.0
Distro: Ubuntu 16.04 LTS
Change-Id: Iaafe030b7262c5fb162afe7118ae592a1a759a58
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26990
Reviewed-by: Gabe Black
Tested-by: kokoro
---
M src/arch/arm/aapcs32.hh
M src/arch/arm/aapcs64.hh
2 files changed, 91 insertions(+), 27 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved
Giacomo Travaglini: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index bfeb553..fd63483 100644
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -131,39 +131,78 @@
template
struct Result::value>::type>
+std::is_integral::value && (sizeof(Integer) <
sizeof(uint32_t))
+>::type>
{
static void
store(ThreadContext *tc, const Integer &i)
{
-if (sizeof(Integer) < sizeof(uint32_t)) {
-uint32_t val = std::is_signed::value ?
-sext(i) : i;
-tc->setIntReg(ArmISA::INTREG_R0, val);
-} else if (sizeof(Integer) == sizeof(uint32_t) ||
- std::is_same::value) {
+uint32_t val = std::is_signed::value ?
+sext(i) : i;
+tc->setIntReg(ArmISA::INTREG_R0, val);
+}
+};
+
+template
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint32_t))
+>::type>
+{
+static void
+store(ThreadContext *tc, const Integer &i)
+{
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
+}
+};
+
+template
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint64_t))
+>::type>
+{
+static void
+store(ThreadContext *tc, const Integer &i)
+{
+if (std::is_same::value) {
tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
-} else if (sizeof(Integer) == sizeof(uint64_t)) {
-if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
-tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
-tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
-} else {
-tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
-tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
-}
+} else if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
+tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
+} else {
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
+tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
}
}
};
template
struct Argument::value>::type> : public Aapcs32ArgumentBase
+std::is_integral::value && (sizeof(Integer) <=
sizeof(uint32_t))
+>::type> : public Aapcs32ArgumentBase
{
static Integer
get(ThreadContext *tc, Aapcs32::State &state)
{
-if ((sizeof(Integer) <= sizeof(uint32_t) ||
-std::is_same::value) &&
+if (state.ncrn <= state.MAX_CRN) {
+return tc->readIntReg(state.ncrn++);
+}
+
+// Max out the ncrn since we effectively exhausted it.
+state.ncrn = state.MAX_CRN + 1;
+
+return loadFromStack(tc, state);
+}
+};
+
+template
+struct Argument+std::is_integral::value && (sizeof(Integer) >
sizeof(uint32_t))
+>::type> : public Aapcs32ArgumentBase
+{
+static Integer
+get(ThreadContext *tc, Aapcs32::State &state)
+{
+if (std::is_same::value &&
state.ncrn <= state.MAX_CRN) {
return tc->readIntReg(state.ncrn++);
}
diff --git a/src/arch/arm/
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix aapcs32/aapcs64 compilation issues
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/26990 )
Change subject: arch-arm: Fix aapcs32/aapcs64 compilation issues
..
arch-arm: Fix aapcs32/aapcs64 compilation issues
Some modern compilers won't build ARM due to how guest ABI
has been implemented.
The error is: "left shift count >= width of type"
[-Werror=shift-count-overflow]
The error is triggered when there is a left shift > the variable size
(in bits); this leads to undefined behaviour.
This is a compile time vs run time problem; the code is technically
fine, but the compiler is not able to understand this.
For example in aapcs64:
struct Argument::value>::type> : public Aapcs64ArgumentBase
{
[...]
if (sizeof(Integer) == 16 && state.ngrn + 1 <= state.MAX_GRN) {
Integer low = tc->readIntReg(state.ngrn++);
Integer high = tc->readIntReg(state.ngrn++);
high = high << 64;
return high | low;
}
}
Even if the sizeof operator will be evaluated at compile time,
the block will be executed at runtime: the block will still be part of
the code if Integer = uint32_t.
The compiler will then throw an error because we are left shifting an
uint32_t by 64 bits.
Change-Id: Iaafe030b7262c5fb162afe7118ae592a1a759a58
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/aapcs32.hh
M src/arch/arm/aapcs64.hh
2 files changed, 91 insertions(+), 27 deletions(-)
diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index bfeb553..fd63483 100644
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -131,39 +131,78 @@
template
struct Result::value>::type>
+std::is_integral::value && (sizeof(Integer) <
sizeof(uint32_t))
+>::type>
{
static void
store(ThreadContext *tc, const Integer &i)
{
-if (sizeof(Integer) < sizeof(uint32_t)) {
-uint32_t val = std::is_signed::value ?
-sext(i) : i;
-tc->setIntReg(ArmISA::INTREG_R0, val);
-} else if (sizeof(Integer) == sizeof(uint32_t) ||
- std::is_same::value) {
+uint32_t val = std::is_signed::value ?
+sext(i) : i;
+tc->setIntReg(ArmISA::INTREG_R0, val);
+}
+};
+
+template
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint32_t))
+>::type>
+{
+static void
+store(ThreadContext *tc, const Integer &i)
+{
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
+}
+};
+
+template
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint64_t))
+>::type>
+{
+static void
+store(ThreadContext *tc, const Integer &i)
+{
+if (std::is_same::value) {
tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
-} else if (sizeof(Integer) == sizeof(uint64_t)) {
-if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
-tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
-tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
-} else {
-tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
-tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
-}
+} else if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
+tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
+} else {
+tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
+tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
}
}
};
template
struct Argument::value>::type> : public Aapcs32ArgumentBase
+std::is_integral::value && (sizeof(Integer) <=
sizeof(uint32_t))
+>::type> : public Aapcs32ArgumentBase
{
static Integer
get(ThreadContext *tc, Aapcs32::State &state)
{
-if ((sizeof(Integer) <= sizeof(uint32_t) ||
-std::is_same::value) &&
+if (state.ncrn <= state.MAX_CRN) {
+return tc->readIntReg(state.ncrn++);
+}
+
+// Max out the ncrn since we effectively exhausted it.
+state.ncrn = state.MAX_CRN + 1;
+
+return loadFromStack(tc, state);
+}
+};
+
+template
+struct Argument+std::is_integral::value && (sizeof(Integer) >
sizeof(uint32_t))
+>::type> : public Aapcs32ArgumentBase
+{
+static Integer
+get(ThreadContext *tc, Aapcs32::State &state)
+{
+if (std::is_same::value &&
state.ncrn <= state.MAX_CRN) {
return tc->readIntReg(state.ncrn++);
}
diff --git a/src/arch/arm/aapcs64.hh b/src/arch/arm/aapcs64.hh
index 9d7623a..203846d 100644
--- a/src/arch/arm/aapcs64.hh
+++ b/src/arch/arm/aapcs64.hh
@@ -229,14 +229,30 @@
// This will pick up Addr as well, which should be used for guest pointers.
template
struct Argument::value>::type> : public Aapcs64Argume
