[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v3 00/34] Hexagon patch series
From: |
Richard Henderson |
Subject: |
Re: [RFC PATCH v3 00/34] Hexagon patch series |
Date: |
Mon, 31 Aug 2020 13:43:54 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
On 8/31/20 10:57 AM, Taylor Simpson wrote:
> OK, here's the list of items. Let me know if I missed anything. I'll
> indicate which ones can be done quickly and which ones would take more time.
> I added a column for blocker if you or anyone else has input on that.
>
> PatchItemEffortBlocker
> Use qemu softfloat??Yes
Hmm, this table didn't render. Below, yes/no for blocker column.
> Use qemu decodetree.py??
no
> SeveralUse const when appropriatesmall
yes
> SeveralRemove anything after g_assert_not_reachedsmall
yes
> SeveralFix log_store32/64 add/remove/add in patch seriessmall
yes
> SeveralFollow naming guidelines for structs and enumssmall
yes
> 04Move decls to cpu-param.hsmall
Yes. The only reason this even compiled is that you don't do system mode yet.
;-)
> 04Remove CONFIG_USER_ONLY ifdef'ssmall
yes
> 04Remove DEBUG_HEXAGONsmall
yes
> 04Remove stack pointer modification hack, use qemu mechanismsmall
yes
> 04Add property x-lldb-compat to control output in logsmall
yes
> 06Include instruction and raw bytes in disassemblysmall
yes
> 07Use DEF_HELPER_FLAGSsmall
no, but should be easy.
> 07, 26Endianness of merge_bytessmall
Yes, one way or another; hopefully by removing all of the merge_bytes and using
probe_read.
> 07Fix overlap testsmall
yes
> 07Remove HELPER(debug_value)/HELPER(debug_value_i64)small
yes
> 09Include "qemu/osdep.h" instead of <stdint.h>small
yes
> 10 (and others)Stick with stdint.h types except in imported filessmall
yes
> 11Remove description from reg field definitionssmall
yes
> 13Move regmap.h into decode.csmall
yes
> 14, 27Use bit mask instead of strings in decodingsmall
no, but should be easy.
> 14Add comments to decodersmall
yes
> 16Use qemu/int128.hmedium
no
> 17Squash patches dealing with imported filessmall
yes
> 24Use qemu/bitops.h for instruction attributessmall
no
> 24Fix initialization of opcode_short_semanticssmall
yes
> 24Change if (p == NULL) { g_assert_not_reached(); } to assert(p != NULL)small
no
> 25Expand DECL/READ/WRITE/FREE macros into generated codesmall
Yes.
In the end I think some of these will in the end want to be helper functions.
As I was thinking how to best write A2_add, I was thinking
/* TODO: You currently have an "offset" parameter to
DECL_REG. I can't see that it is ever used?
I would *hope* that this could be resolved earlier,
so that by this time insn->regno[*] is absolute. */
static int resolve_regno(Insn *insn, int slot, int off);
/* Return hex_new_value[regno]; log the write. */
static TCGv reg_for_write(DisasContext *ctx, int regno);
/* Return hex_reg[regno]; force up-to-date value for PC. */
static TCGv reg_for_read(DisasContext *ctx, int regno);
/* if (preg) hex_new_value[regno] = hex_reg[regno],
or !preg if !test_positive.
Leaves hex_new_value[] canonical for gen_reg_writes,
no extra temporary required. */
static void gen_cancel_reg(DisasContext *ctx, int preg,
int rreg, bool test_positive);
DEF_TCG_FUNC(A2_add)
{
int rd = resolve_regno(insn, 0, 0);
int rs = resolve_regno(insn, 1, 0);
int rt = resolve_regno(insn, 2, 0);
tcg_gen_add_tl(reg_for_write(ctx, rd),
reg_for_read(ctx, rs),
reg_for_read(ctx, rt));
}
DEF_TCG_FUNC(A2_paddit)
{
int pu = resolve_regno(insn, 0, 0);
int rd = resolve_regno(insn, 1, 0);
int rs = resolve_regno(insn, 2, 0);
int rt = resolve_regno(insn, 3, 0);
tcg_gen_add_tl(reg_for_write(ctx, rd),
reg_for_read(ctx, rs),
reg_for_read(ctx, rt));
gen_cancel_reg(ctx, insn, rd, pu, true);
}
However, I don't think we have to have a comprehensive set of these now. We
can expand everything into the generator to start, then adjust the generator as
we add helper functions and use them within the overrides.
> 26Rewrite fINSERT*, fEXTRACT*, f?XTN macrossmall
yes
> 26Investigate fRND macrosmall
no, but should be easy.
> 26Change REG = REG to (VOID)REG to suppress compiler warningsmall
yes
> 27Remove multiple includes of imported/iclass.defsmall
yes
> 28Move genptr_helpers.h into genptr.csmall
yes
> 28Remove unneeded tempssmall
no
> 28Use tcg_gen_deposit_tl and tcg_gen_extract_tl when dealing with p3_0small
no
> 29Size opcode_genptr[] properly and initialize with [TAG] =
> generate_##TAGsmall
yes; i think this will fall out of other changes.
> 30Don't generate helpers for instructions that are overriddensmall
yes
> Don't include "gen_tcg.h" in helper.h
yes
> 31Use bitmask for ctx->reg_log instead of an arraysmall
yes
> 31Use tcg_gen_extract_i32 for gen_slot_cancelled_checksmall
yes
> 31Properly deal with reading instructions across a page boundary and toomedium
> many instructions before finding end-of-packet
Yes, this should be easy. Unless there's some surprise in the code I have
already suggested code.
> 31Don't set PC at the beginning of every packetmedium
no
> 31Don't set slot_cancelled unless neededsmall
no
> 31Don't set hex_pred_written unless neededmedium
no
> 31Change cancelled variable to not localsmall
yes
> 31Remove unnecessary tempsmall
yes
> 31Let tcg_gen_addi_tl check for zerosmall
yes
> 31Move gen_exec_counters to end of TB instead of every packetmedium
no
> 31Move end of TB handling to hexagon_tr_tb_stopsmall
yes
r~
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, (continued)
[RFC PATCH v3 33/34] Hexagon (tests/tcg/hexagon) TCG tests, Taylor Simpson, 2020/08/18
Re: [RFC PATCH v3 00/34] Hexagon patch series, no-reply, 2020/08/18
Re: [RFC PATCH v3 00/34] Hexagon patch series, Richard Henderson, 2020/08/28