| Message ID | 20210617174907.2904067-2-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/mips: Extract microMIPS ISA and Code Compaction ASE apart | expand |
On 6/17/21 10:49 AM, Philippe Mathieu-Daudé wrote: > To be able to extract the microMIPS ISA and Code Compaction ASE > translation routines to different source files, declare few TCG > helpers which are also used by translate.c in "translate.h". > > Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org> > --- > target/mips/tcg/translate.h | 5 +++++ > target/mips/tcg/translate.c | 9 ++++----- > 2 files changed, 9 insertions(+), 5 deletions(-) What patch set does this belong with? It doesn't seem to match the next two patches, which don't create new compilation units. As far as it goes, the patch is fine... Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 6/18/21 10:15 PM, Richard Henderson wrote: > On 6/17/21 10:49 AM, Philippe Mathieu-Daudé wrote: >> To be able to extract the microMIPS ISA and Code Compaction ASE >> translation routines to different source files, declare few TCG >> helpers which are also used by translate.c in "translate.h". >> >> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org> >> --- >> target/mips/tcg/translate.h | 5 +++++ >> target/mips/tcg/translate.c | 9 ++++----- >> 2 files changed, 9 insertions(+), 5 deletions(-) > > What patch set does this belong with? micromips_translate.c.inc:1778: gen_ldxs(ctx, rs, rt, rd); micromips_translate.c.inc:1806: gen_align(ctx, 32, rd, rs, rt, extract32(ctx->opcode, 9, 2)); micromips_translate.c.inc:2859: gen_addiupc(ctx, reg, offset, 0, 0); mips16e_translate.c.inc:444: gen_addiupc(ctx, ry, offset, 1, extended); mips16e_translate.c.inc:481: gen_addiupc(ctx, rx, imm, 0, 1); mips16e_translate.c.inc:682: gen_addiupc(ctx, rx, ((uint8_t) ctx->opcode) << 2, 0, 0); > It doesn't seem to match the next > two patches, which don't create new compilation units. Indeed. In a yet-to-be-posted later series they get renamed to .c, becoming new units. It seemed simpler for me to do it that way, but I'll see how to rearrange the patches. Ah, now I remember, rearranging the patches modify the next 2 big patches (total 4k+ lines moved) you already reviewed... So to keep your two R-b it is simpler to add this one first. What I can do is hold these patches and post them later with the yet-to-be-posted series, *but* that breaks the "extract nanomips from translate.c" which you already reviewed :/ It is hard to split this huge 25k+ spaghetti translate.c :( > As far as it goes, the patch is fine... > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~ >
On 6/19/21 2:26 AM, Philippe Mathieu-Daudé wrote: >> It doesn't seem to match the next >> two patches, which don't create new compilation units. > > Indeed. In a yet-to-be-posted later series they get renamed to .c, > becoming new units. Ah, ok. > What I can do is hold these patches and post them later with the > yet-to-be-posted series, *but* that breaks the "extract nanomips > from translate.c" which you already reviewed :/ I don't mind the ordering, just that there's a reason. > It is hard to split this huge 25k+ spaghetti translate.c :( I can well imagine. r~
Hi Richard, On 6/19/21 2:41 PM, Richard Henderson wrote: > On 6/19/21 2:26 AM, Philippe Mathieu-Daudé wrote: >>> It doesn't seem to match the next >>> two patches, which don't create new compilation units. >> >> Indeed. In a yet-to-be-posted later series they get renamed to .c, >> becoming new units. > > Ah, ok. > >> What I can do is hold these patches and post them later with the >> yet-to-be-posted series, *but* that breaks the "extract nanomips >> from translate.c" which you already reviewed :/ > > I don't mind the ordering, just that there's a reason. Is that OK if I reword as: --- We want to extract the microMIPS ISA and Code Compaction ASE to new compilation units. We will first extract this code as included source files (.c.inc), then make them new compilation units afterward. The following methods are going to be used externally: micromips_translate.c.inc:1778: gen_ldxs(ctx, rs, rt, rd); micromips_translate.c.inc:1806: gen_align(ctx, 32, rd, rs, ... micromips_translate.c.inc:2859: gen_addiupc(ctx, reg, offset, ... mips16e_translate.c.inc:444: gen_addiupc(ctx, ry, offset, ... To avoid too much code churn, it is simpler to declare these prototypes in "translate.h" now. --- ?
On 6/28/21 9:33 AM, Philippe Mathieu-Daudé wrote: >> I don't mind the ordering, just that there's a reason. > > Is that OK if I reword as: > > --- > We want to extract the microMIPS ISA and Code Compaction ASE to > new compilation units. > > We will first extract this code as included source files (.c.inc), > then make them new compilation units afterward. > > The following methods are going to be used externally: > > micromips_translate.c.inc:1778: gen_ldxs(ctx, rs, rt, rd); > micromips_translate.c.inc:1806: gen_align(ctx, 32, rd, rs, ... > micromips_translate.c.inc:2859: gen_addiupc(ctx, reg, offset, ... > mips16e_translate.c.inc:444: gen_addiupc(ctx, ry, offset, ... > > To avoid too much code churn, it is simpler to declare these > prototypes in "translate.h" now. > --- Sure. r~
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h index 61442590340..c25fad597d5 100644 --- a/target/mips/tcg/translate.h +++ b/target/mips/tcg/translate.h @@ -146,6 +146,11 @@ void gen_store_fpr32(DisasContext *ctx, TCGv_i32 t, int reg); void gen_store_fpr64(DisasContext *ctx, TCGv_i64 t, int reg); int get_fp_bit(int cc); +void gen_ldxs(DisasContext *ctx, int base, int index, int rd); +void gen_align(DisasContext *ctx, int wordsz, int rd, int rs, int rt, int bp); +void gen_addiupc(DisasContext *ctx, int rx, int imm, + int is_64_bit, int extended); + /* * Address Computation and Large Constant Instructions */ diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c index 255f97fe9c1..c0ae180969e 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -5630,8 +5630,7 @@ static void gen_align_bits(DisasContext *ctx, int wordsz, int rd, int rs, tcg_temp_free(t0); } -static void gen_align(DisasContext *ctx, int wordsz, int rd, int rs, int rt, - int bp) +void gen_align(DisasContext *ctx, int wordsz, int rd, int rs, int rt, int bp) { gen_align_bits(ctx, wordsz, rd, rs, rt, bp * 8); } @@ -12779,8 +12778,8 @@ static void gen_mips16_restore(DisasContext *ctx, tcg_temp_free(t2); } -static void gen_addiupc(DisasContext *ctx, int rx, int imm, - int is_64_bit, int extended) +void gen_addiupc(DisasContext *ctx, int rx, int imm, + int is_64_bit, int extended) { TCGv t0; @@ -14511,7 +14510,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx) } } -static void gen_ldxs(DisasContext *ctx, int base, int index, int rd) +void gen_ldxs(DisasContext *ctx, int base, int index, int rd) { TCGv t0 = tcg_temp_new(); TCGv t1 = tcg_temp_new();
To be able to extract the microMIPS ISA and Code Compaction ASE translation routines to different source files, declare few TCG helpers which are also used by translate.c in "translate.h". Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/mips/tcg/translate.h | 5 +++++ target/mips/tcg/translate.c | 9 ++++----- 2 files changed, 9 insertions(+), 5 deletions(-)