[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 2/3] tcg/optimize: do copy propagation for m
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH RFC 2/3] tcg/optimize: do copy propagation for memory locations |
Date: |
Wed, 22 Nov 2017 09:44:14 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/09/2017 03:41 PM, Kirill Batuzov wrote:
> +typedef struct TCGMemLocation {
> + /* Offset is relative to ENV. Only fields of CPUState are accounted. */
> + tcg_target_ulong offset;
> + tcg_target_ulong size;
> + TCGType type;
> + /* Pointer to a temp containing a valid copy of this memory location. */
> + TCGTemp *copy;
> + /* Pointer to the next memory location containing copy of the same
> + content. */
> + struct TCGMemLocation *next_copy;
Did you ever find copies of memories that weren't also copies within temps?
I.e. you could have found this through copy->next_copy?
> + /* Double-linked list of all memory locations. */
> + struct TCGMemLocation *next;
> + struct TCGMemLocation **prev_ptr;
Use QTAILQ_* for common double-linked-list manipulation.
> +struct TCGMemLocation *mem_locations;
> +struct TCGMemLocation *free_mls;
These can't be globals anymore -- we're do multi-threaded code generation now.
> @@ -77,12 +125,27 @@ static void reset_ts(TCGTemp *ts)
> struct tcg_temp_info *pi = ts_info(ti->prev_copy);
> struct tcg_temp_info *ni = ts_info(ti->next_copy);
>
> + if (ti->mem_loc && ts_is_copy(ts) && 0) {
> + TCGMemLocation *ml, *nml;
> + for (ml = ti->mem_loc; ml; ml = nml) {
> + nml = ml->next_copy;
> + ml->copy = ti->next_copy;
> + ml->next_copy = ni->mem_loc;
> + ni->mem_loc = ml;
> + }
> + } else {
> + while (ti->mem_loc) {
> + reset_ml(ti->mem_loc);
> + }
Why would a single temp be associated with more than one memory?
> +static TCGOpcode ld_to_mov(TCGOpcode op)
> +{
> +#define LD_TO_EXT(sz, w) \
> + case glue(glue(INDEX_op_ld, sz), glue(_i, w)): \
> + return glue(glue(INDEX_op_ext, sz), glue(_i, w))
> +
> + switch (op) {
> + LD_TO_EXT(8s, 32);
> + LD_TO_EXT(8u, 32);
> + LD_TO_EXT(16s, 32);
> + LD_TO_EXT(16u, 32);
> + LD_TO_EXT(8s, 64);
> + LD_TO_EXT(8u, 64);
> + LD_TO_EXT(16s, 64);
> + LD_TO_EXT(16u, 64);
> + LD_TO_EXT(32s, 64);
> + LD_TO_EXT(32u, 64);
How many extensions did you find? Or is this Just In Case?
Otherwise this looks quite reasonable.
r~