[PATCH 20/27] arcv3: TCG, decoder glue code and helper changes
Richard Henderson
richard.henderson at linaro.org
Thu Apr 8 00:36:34 BST 2021
On 4/5/21 7:31 AM, cupertinomiranda at gmail.com wrote:
> +uint64_t helper_carry_add_flag32(uint64_t dest, uint64_t b, uint64_t c) {
> + return carry_add_flag(dest, b, c, 32);
> +}
> +
> +target_ulong helper_overflow_add_flag32(target_ulong dest, target_ulong b, target_ulong c) {
> + return overflow_add_flag(dest, b, c, 32);
> +}
> +
> +target_ulong helper_overflow_sub_flag32(target_ulong dest, target_ulong b, target_ulong c) {
> + dest = dest & 0xffffffff;
> + b = b & 0xffffffff;
> + c = c & 0xffffffff;
> + return overflow_sub_flag(dest, b, c, 32);
> +}
You shouldn't need to replicate these functions. Use the correct types and
masking in the first place.
> +uint64_t helper_rotate_left32(uint64_t orig, uint64_t n)
> +{
> + uint64_t t;
> + uint64_t dest = (orig << n) & ((0xffffffff << n) & 0xffffffff);
> +
> + t = (orig >> (32 - n)) & ((1 << n) - 1);
> + dest |= t;
> +
> + return dest;
> +}
> +
> +uint64_t helper_rotate_right32(uint64_t orig, uint64_t n)
> +{
> + uint64_t t;
> + uint64_t dest = (orig >> n) & (0xffffffff >> n);
> +
> + t = ((orig & ((1 << n) - 1)) << (32 - n));
> + dest |= t;
> +
> + return dest;
> +}
rol32 and ror32.
> +uint64_t helper_asr_32(uint64_t b, uint64_t c)
> +{
> + uint64_t t;
> + c = c & 31;
> + t = b;
> + for(int i = 0; i < c; i++) {
> + t >>= 1;
> + if((b & 0x80000000) != 0)
> + t |= 0x80000000;
> + }
> + //t |= ((1 << (c+1)) - 1) << (32 - c);
> +
> + return t;
Really? I can't imagine what lead you to write this.
Who writes a simple shift operation with a loop?
Perhaps no helper at all and
tcg_gen_sra_tl(ret, b, c);
tcg_gen_ext32s_tl(ret, ret);
> +target_ulong helper_ffs32(CPUARCState *env, uint64_t src)
> +{
> + int i;
> + if (src == 0) {
> + return 31;
> + }
> + for (i = 0; i <= 31; i++) {
> + if (((src >> i) & 1) != 0) {
> + break;
> + }
> + }
> + return i;
> +}
tcg_gen_ori_tl(ret, src, MAKE_64BIT_MASK(32, 32));
tcg_gen_ctzi_tl(ret, ret, 31);
Though I really wonder if you've got that function correct, as it's not the
*normal* definition of ffs...
> +target_ulong helper_norml(CPUARCState *env, uint64_t src1)
> +{
> + int i;
> + int64_t tmp = (int64_t) src1;
> + if (tmp == 0 || tmp == -1) {
> + return 0;
> + }
> + for (i = 0; i <= 63; i++) {
> + if ((tmp >> i) == 0) {
> + break;
> + }
> + if ((tmp >> i) == -1) {
> + break;
> + }
> + }
> + return i;
> +}
This is some cognate of count-leading-repititions-of-sign-bit,
tcg_gen_clrsb_tl. A decent computation should be like
tcg_gen_clrsb_i64(ret, src);
tcg_gen_subfi_i64(ret, 63, ret);
> diff --git a/target/arc/semfunc-v2_mapping.def b/target/arc/semfunc-v2_mapping.def
> new file mode 100644
> index 0000000000..ab8d9ff123
> --- /dev/null
> +++ b/target/arc/semfunc-v2_mapping.def
You could have named this properly to start.
r~
More information about the linux-snps-arc
mailing list