Hi, On 12 December 2011 16:55, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 12 December 2011 15:37, Dr. David Alan Gilbert > <david.gilb...@linaro.org> wrote: >> On ARM, don't map the code buffer at a fixed location, and fix up the >> call/goto tcg routines to let it do long jumps. >> >> Mapping the code buffer at a fixed address could sometimes result in it being >> mapped over the top of the heap with pretty random results. >> >> This diff is against v1.0. > > Patches should always be against master, although we're still > close enough to 1.0 that this will probably apply anyway. > > Comments like "This diff is against v1.0." go below the '---' so they > don't appear in the final commit log. > > Code generally looks OK to me.
To me as well, I'll apply if there are no objections. I remember Peter asked about MAP_FIXED on other hosts, has there been a resolution? > >> Signed-off-by: Dr. David Alan Gilbert <david.gilb...@linaro.org> >> --- >> exec.c | 4 +--- >> tcg/arm/tcg-target.c | 31 ++++++++++++------------------- >> 2 files changed, 13 insertions(+), 22 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 6b92198..ef83da1 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -497,9 +497,7 @@ static void code_gen_alloc(unsigned long tb_size) >> if (code_gen_buffer_size > (512 * 1024 * 1024)) >> code_gen_buffer_size = (512 * 1024 * 1024); >> #elif defined(__arm__) >> - /* Map the buffer below 32M, so we can use direct calls and >> branches */ >> - flags |= MAP_FIXED; >> - start = (void *) 0x01000000UL; >> + /* Keep the buffer no bigger than 16GB to branch between blocks */ >> if (code_gen_buffer_size > 16 * 1024 * 1024) >> code_gen_buffer_size = 16 * 1024 * 1024; >> #elif defined(__s390x__) >> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c >> index e05a64f..730d913 100644 >> --- a/tcg/arm/tcg-target.c >> +++ b/tcg/arm/tcg-target.c >> @@ -842,6 +842,12 @@ static inline void tcg_out_st8(TCGContext *s, int cond, >> tcg_out_st8_12(s, cond, rd, rn, offset); >> } >> >> +/* The _goto case is normally between TBs within the same code buffer, >> + and with the code buffer limited to 16GB we shouldn't need the long >> + case. >> + >> + .... except to the prologue that is in its own buffer. >> + */ >> static inline void tcg_out_goto(TCGContext *s, int cond, uint32_t addr) >> { >> int32_t val; >> @@ -855,22 +861,20 @@ static inline void tcg_out_goto(TCGContext *s, int >> cond, uint32_t addr) >> if (val - 8 < 0x01fffffd && val - 8 > -0x01fffffd) >> tcg_out_b(s, cond, val); >> else { >> -#if 1 >> - tcg_abort(); >> -#else >> if (cond == COND_AL) { >> tcg_out_ld32_12(s, COND_AL, TCG_REG_PC, TCG_REG_PC, -4); >> - tcg_out32(s, addr); /* XXX: This is l->u.value, can we use it? >> */ >> + tcg_out32(s, addr); > > I see these XXXs have been there since the ARM tcg target was > introduced. Does anybody know what they were referring to? Andrzej? David asked me about them, but I couldn't figure it out. (the part in tcg_out_call apparently is just copy&pasted from tcg_ouy_goto) BTW: I think we can also use the "ld" branch when we see the goto target is in Thumb mode. Cheers