From 2fbd03599d2c0a7e8ce149cba173dfdeea1d9975 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Fri, 20 Mar 2020 17:29:52 +0000 Subject: [PATCH] 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 --- src/arch/arm/aapcs32.hh | 75 +++++++++++++++++++++++++++++++---------- src/arch/arm/aapcs64.hh | 43 ++++++++++++++++++----- 2 files changed, 91 insertions(+), 27 deletions(-) diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh index bfeb55374..fd63483ef 100644 --- a/src/arch/arm/aapcs32.hh +++ b/src/arch/arm/aapcs32.hh @@ -131,39 +131,78 @@ struct Aapcs32ArgumentBase 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::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::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 (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::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 (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 9d7623a05..203846d43 100644 --- a/src/arch/arm/aapcs64.hh +++ b/src/arch/arm/aapcs64.hh @@ -229,14 +229,30 @@ struct Result struct Argument::value>::type> : public Aapcs64ArgumentBase + std::is_integral::value && (sizeof(Integer) <= 8) + >::type> : public Aapcs64ArgumentBase { static Integer get(ThreadContext *tc, Aapcs64::State &state) { - if (sizeof(Integer) <= 8 && state.ngrn <= state.MAX_GRN) + if (state.ngrn <= state.MAX_GRN) return tc->readIntReg(state.ngrn++); + // Max out ngrn since we've effectively saturated it. + state.ngrn = state.MAX_GRN + 1; + + return loadFromStack(tc, state); + } +}; + +template +struct Argument::value && (sizeof(Integer) > 8) + >::type> : public Aapcs64ArgumentBase +{ + static Integer + get(ThreadContext *tc, Aapcs64::State &state) + { if (alignof(Integer) == 16 && (state.ngrn % 2)) state.ngrn++; @@ -256,17 +272,26 @@ struct Argument struct Result::value>::type> + std::is_integral::value && (sizeof(Integer) <= 8) + >::type> { static void store(ThreadContext *tc, const Integer &i) { - if (sizeof(Integer) <= 8) { - tc->setIntReg(0, i); - } else { - tc->setIntReg(0, (uint64_t)i); - tc->setIntReg(1, (uint64_t)(i >> 64)); - } + tc->setIntReg(0, i); + } +}; + +template +struct Result::value && (sizeof(Integer) > 8) + >::type> +{ + static void + store(ThreadContext *tc, const Integer &i) + { + tc->setIntReg(0, (uint64_t)i); + tc->setIntReg(1, (uint64_t)(i >> 64)); } }; -- 2.30.2