[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers
From: |
Richard Henderson |
Subject: |
Re: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers |
Date: |
Wed, 26 Aug 2020 07:16:32 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
On 8/18/20 8:50 AM, Taylor Simpson wrote:
> The majority of helpers are generated. Define the helper functions needed
> then include the generated file
>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
> target/hexagon/helper.h | 33 ++++
> target/hexagon/op_helper.c | 368
> +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 401 insertions(+)
> create mode 100644 target/hexagon/helper.h
> create mode 100644 target/hexagon/op_helper.c
>
> diff --git a/target/hexagon/helper.h b/target/hexagon/helper.h
> new file mode 100644
> index 0000000..48b1917
> --- /dev/null
> +++ b/target/hexagon/helper.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright(c) 2019-2020 Qualcomm Innovation Center, Inc. All Rights
> Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +DEF_HELPER_2(raise_exception, noreturn, env, i32)
> +DEF_HELPER_1(debug_start_packet, void, env)
> +DEF_HELPER_3(debug_check_store_width, void, env, int, int)
> +DEF_HELPER_3(debug_commit_end, void, env, int, int)
> +DEF_HELPER_3(merge_inflight_store1s, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store1u, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store2s, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store2u, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store4s, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store4u, s32, env, s32, s32)
> +DEF_HELPER_3(merge_inflight_store8u, s64, env, s32, s64)
Please use DEF_HELPER_FLAGS_N when possible.
You should be able to know when
(1) No exceptions are possible, and nothing is touched except the inputs. In
this case use TCG_CALL_NO_RWG_SE.
(2) No exceptions are possible, and nothing in env is touched for which you
have created a tcg variable in hexagon_translate_init(). In this case use
TCG_CALL_NO_RWG.
(3) Exceptions are possible, and no tcg variables are touched on the
non-exceptional path. In this case use TCG_CALL_NO_WG.
> +static inline void log_pred_write(CPUHexagonState *env, int pnum,
> + target_ulong val)
You might be better off letting the compiler decide about inlining.
> +/*
> + * Handle mem_noshuf
> + *
> + * This occurs when there is a load that might need data forwarded
> + * from an inflight store in slot 1. Note that the load and store
> + * might have different sizes, so we can't simply compare the
> + * addresses. We merge only the bytes that overlap (if any).
> + */
> +static int64_t merge_bytes(CPUHexagonState *env, target_ulong load_addr,
> + int64_t load_data, uint32_t load_width)
> +{
> + /* Don't do anything if slot 1 was cancelled */
> + const int store_slot = 1;
> + if (env->slot_cancelled & (1 << store_slot)) {
> + return load_data;
> + }
> +
> + int store_width = env->mem_log_stores[store_slot].width;
> + target_ulong store_addr = env->mem_log_stores[store_slot].va;
> + union {
> + uint8_t bytes[8];
> + uint32_t data32;
> + uint64_t data64;
> + } retdata, storedata;
Red flag here. This is assuming that the host and the target are both
little-endian.
> + int bigmask = ((-load_width) & (-store_width));
> + if ((load_addr & bigmask) != (store_addr & bigmask)) {
Huh? This doesn't detect overlap. Counter example:
load_addr = 0, load_width = 4,
store_addr = 1, store_width = 1.
bigmask = -4 & -1 -> -4.
(0 & -4) != (1 & -4) -> 0 != 1
> + while ((i < load_width) && (j < store_width)) {
> + retdata.bytes[i] = storedata.bytes[j];
> + i++;
> + j++;
> + }
> + return retdata.data64;
This most definitely requires host-endian adjustment.
> +/* Helpful for printing intermediate values within instructions */
> +void HELPER(debug_value)(CPUHexagonState *env, int32_t value)
> +{
> + HEX_DEBUG_LOG("value = 0x%x\n", value);
> +}
> +
> +void HELPER(debug_value_i64)(CPUHexagonState *env, int64_t value)
> +{
> + HEX_DEBUG_LOG("value_i64 = 0x%lx\n", value);
> +}
I think we need to lose all of this.
There are other ways to debug TCG.
r~
- Re: [RFC PATCH v3 02/34] Hexagon (target/hexagon) README, (continued)
- [RFC PATCH v3 03/34] Hexagon (include/elf.h) ELF machine definition, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 13/34] Hexagon (target/hexagon) register map, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 14/34] Hexagon (target/hexagon) instruction/packet decode, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers, Taylor Simpson, 2020/08/18
- Re: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers,
Richard Henderson <=
- [RFC PATCH v3 17/34] Hexagon (target/hexagon/imported) arch import - macro definitions, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 23/34] Hexagon (target/hexagon) generater phase 4 - decode tree, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 09/34] Hexagon (target/hexagon) architecture types, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 16/34] Hexagon (target/hexagon) utility functions, Taylor Simpson, 2020/08/18