[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm an
From: |
Claudio Fontana |
Subject: |
Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl |
Date: |
Wed, 18 Sep 2013 10:24:21 +0200 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
On 17.09.2013 16:54, Richard Henderson wrote:
> On 09/17/2013 01:23 AM, Claudio Fontana wrote:
>>> It would have been handy if ARM had officially assigned identifiers to the
>>> formats, like Power, S390, and ia64 do. Then one can build in the format
>>> ids
>>> into both the function and enumeration names and use the preprocessor for
>>> typechecking (c.f. the tcg_out_insn macro in tcg/s390/tcg-target.c).
>>
>> No need to do force explicit typechecking like that.
>> That kind of use of the preprocessor really hurts.
>
> Why do you believe this? Have you browsed through the s390 backend?
> I think it's a remarkably clean solution -- one we ought to have used
> in the ia64 backend, which has even more format codes.
Why do I believe this? Because my experience tells me to let this kind of stuff
go,
in order to allow developers that are not familiar with the code base yet to
trace their way through the calls.
It keeps the code base discoverable, by hand and by tools.
>>> Therefore I think adding LSR, ASR and ROR shifts is both a waste of time and
>>> bloats the backend.
>>
>> I agree, lets just keep only left shift and right shift.
>> Distinguishing the two costs one comparison per call, I think we can survive
>> it.
>
> I don't understand you at all.
>
> You agree that removing the unused shift from cmp is sensible.
Yes. The additional parameter to cmp (0) can be confusing, therefore it seems
more sensible to remove it until there is actual demonstrable use for the
shifted version of cmp.
> You agree that not adding unused asr/ror shifts is sensible.
Yep.
> But you insist that the unused lsr shift should be retained?
Yes. It's a balance thing, it's not black and white.
In this case the drawback of keeping the right shift is negligible.
No additional parameter is needed, the existing code just looks at the sign of
the immediate to decide the direction to shift.
It's one comparison only, and I find that the tradeoff is acceptable.
If you _really_ want to strip the right shift functionality away for some
reason, could you state it?
Is it for that one comparision? Or for consistency with something else?
>
> You complain about wasting Y space in my positioning of comments.
> But you insist on wasting X space,
They are different resources. Y space gives us context and is a more scarse and
precious resource than X space.
Of course it's a tradeoff.
We don't want the whole code on one line, we have 80 char limits and we do one
thing per line.
> and allowing the possibility of
> mismatch, by requiring format names to be duplicated?
I don't understand this one.
C.
- Re: [Qemu-devel] [PATCH v4 06/33] tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn, (continued)
[Qemu-devel] [PATCH v4 07/33] tcg-aarch64: Remove the shift_imm parameter from tcg_out_cmp, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Richard Henderson, 2013/09/14
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Claudio Fontana, 2013/09/16
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Richard Henderson, 2013/09/16
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Richard Henderson, 2013/09/16
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Claudio Fontana, 2013/09/17
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Richard Henderson, 2013/09/17
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl,
Claudio Fontana <=
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Richard Henderson, 2013/09/18
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, Claudio Fontana, 2013/09/18
[Qemu-devel] [PATCH v4 09/33] tcg-aarch64: Introduce tcg_fmt_Rdn_aimm, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 10/33] tcg-aarch64: Implement mov with tcg_fmt_* functions, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 11/33] tcg-aarch64: Handle constant operands to add, sub, and compare, Richard Henderson, 2013/09/14