[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 11/33] tcg-aarch64: Handle constant operands
From: |
Claudio Fontana |
Subject: |
Re: [Qemu-devel] [PATCH v4 11/33] tcg-aarch64: Handle constant operands to add, sub, and compare |
Date: |
Tue, 17 Sep 2013 10:49:41 +0200 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
On 16.09.2013 17:45, Richard Henderson wrote:
> On 09/16/2013 02:02 AM, Claudio Fontana wrote:
>>> -static inline void tcg_out_cmp(TCGContext *s, TCGType ext, TCGReg rn,
>>> - TCGReg rm)
>>> +static void tcg_out_cmp(TCGContext *s, TCGType ext, TCGReg a,
>>> + tcg_target_long b, bool const_b)
>>> {
>>> - /* Using CMP alias SUBS wzr, Wn, Wm */
>>> - tcg_fmt_Rdnm(s, INSN_SUBS, ext, TCG_REG_XZR, rn, rm);
>>> + if (const_b) {
>>> + /* Using CMP or CMN aliases. */
>>> + AArch64Insn insn = INSN_SUBSI;
>>> + if (b < 0) {
>>> + insn = INSN_ADDSI;
>>> + b = -b;
>>> + }
>>> + tcg_fmt_Rdn_aimm(s, insn, ext, TCG_REG_XZR, a, b);
>>> + } else {
>>> + /* Using CMP alias SUBS wzr, Wn, Wm */
>>> + tcg_fmt_Rdnm(s, INSN_SUBS, ext, TCG_REG_XZR, a, b);
>>> + }
>>> }
>>
>> What about splitting into tcg_out_cmp_imm and tcg_out_cmp_reg? or
>> tcg_out_cmp_i and tcg_out_cmp_r. The function is an 'if else' anyway with no
>> code sharing, and we would avoid sidestepping the TCGReg type check for b in
>> the _r case, as well as the const_b additional param.
>
> This function is called once from tcg_out_tlb_read and three times from
> tcg_out_opc. I just thought since the majority of uses would have to perform
> this if then we might as well have it in the subroutine than force all of the
> callers to replicate it.
Ok, that's a good point.
What about we keep the tcg_out_cmp and we have it do
if (const_b) {
tcg_out_cmp_i();
} else {
tcg_out_cmp_r();
}
so that code that wants to call cmp_r or cmp_i directly can do that?
I realize that at them moment it would benefit only tcg_out_tlb_read's use.
>>> +static void tcg_out_addsubi(TCGContext *s, int ext, TCGReg rd,
>>> + TCGReg rn, int aimm)
>>> +{
>>> + AArch64Insn insn = INSN_ADDI;
>>> + if (aimm < 0) {
>>> + insn = INSN_SUBI;
>>> + aimm = -aimm;
>>> + }
>>> + tcg_fmt_Rdn_aimm(s, insn, ext, rd, rn, aimm);
>>> +}
>>> +
>>
>> Could this be a tcg_out_arith_imm, in the similar way we would do
>> tcg_out_arith? (tcg_out_arith_reg?)
>
> Which one gets renamed? You already proposed tcg_fmt_Rdn_aimm be named
> tcg_out_arith_imm. Now you want tcg_out_addsubi renamed to the same name?
This one confused me, possibly because I don't see the reason for addsubi.
>
> I suppose we could merge the two if we add the S bit as a parameter. Then
> we don't have to distinguish between ADDI and ADDIS, and we could share code
> with comparisons above...
I am ok with keeping the two separate, distinguishing in principle a
subtraction from a comparison.
Claudio
- Re: [Qemu-devel] [PATCH v4 08/33] tcg-aarch64: Introduce tcg_fmt_Rdnm and tcg_fmt_Rdnm_lsl, (continued)
[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
[Qemu-devel] [PATCH v4 12/33] tcg-aarch64: Handle constant operands to and, or, xor, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 13/33] tcg-aarch64: Support andc, orc, eqv, not, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 14/33] tcg-aarch64: Handle zero as first argument to sub, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 15/33] tcg-aarch64: Support movcond, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 16/33] tcg-aarch64: Use tcg_fmt_Rdnm_cond for setcond, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 17/33] tcg-aarch64: Support deposit, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 18/33] tcg-aarch64: Support add2, sub2, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 19/33] tcg-aarch64: Support muluh, mulsh, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 20/33] tcg-aarch64: Support div, rem, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 21/33] tcg-aarch64: Introduce tcg_fmt_Rd_uimm, Richard Henderson, 2013/09/14