[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 04/33] tcg-aarch64: Hoist common argument loa
From: |
Claudio Fontana |
Subject: |
Re: [Qemu-devel] [PATCH v4 04/33] tcg-aarch64: Hoist common argument loads in tcg_out_op |
Date: |
Tue, 17 Sep 2013 10:01:23 +0200 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
On 16.09.2013 18:20, Richard Henderson wrote:
> On 09/16/2013 12:42 AM, Claudio Fontana wrote:
>>> + /* Hoist the loads of the most common arguments. */
>>>> + TCGArg a0 = args[0];
>>>> + TCGArg a1 = args[1];
>>>> + TCGArg a2 = args[2];
>>>> + int c2 = const_args[2];
>>>> +
>> Either all or none (add c0, c1), I would expect the compiler not to
>> generate code for the paths that don't use C[n].
>
> I chose the most common. Those used in 90% of all of the cases.
What you did is clear, does not change the fact that the mixing of use of the
variables and args[] is confusing.
So either we add int c0 and int c1, and use the hoisted variables exclusively
later, or we don't do it.
>> Btw, if the compiler generates bloated code without this, we should notify
>> the projects working on gcc for aarch64.
>
> It's not the compiler's fault. After parameter decomposition, the arrays
> become pointers, and the compiler can't tell that it's always safe to perform
> the loads. So in general it can't hoist the loads higher than the first
> explicit reference that proves the pointers must be non-null.
>
> Now that I think about it, we might actually do better, generically, to
> package
> all of these arguments up into a struct. The compiler can more easily reason
> about the collective safety of structure access...
I don't have anything against it in principle, but just adding c0 and c1, which
iirc should cover all uses, would be fine by me.
Claudio
[Qemu-devel] [PATCH v4 05/33] tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 06/33] tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn, Richard Henderson, 2013/09/14
[Qemu-devel] [PATCH v4 07/33] tcg-aarch64: Remove the shift_imm parameter from tcg_out_cmp, Richard Henderson, 2013/09/14