| Message ID | 20201103033947.94157-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [for-5.2] tcg: Remove assert from set_jmp_reset_offset | expand |
Hi Richard > -----Original Message----- > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Tuesday, November 3, 2020 9:10 AM > To: qemu-devel@nongnu.org > Cc: Sai Pavan Boddu <saipava@xilinx.com> > Subject: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset > > The range check done here is done later, more appropriately, at the end of > tcg_gen_code. There, a failing range check results in a returned error code, > which causes the TB to be restarted at half the size. > > Reported-by: Sai Pavan Boddu <saipava@xilinx.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > > Sai, would you try this against your failing testcase? [Sai Pavan Boddu] Thanks, this patch fixes the test. Thanks for the support. Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> Regards, Sai Pavan > > > r~ > > --- > tcg/tcg.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index f49f1a7f35..43c6cf8f52 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s) > > static void set_jmp_reset_offset(TCGContext *s, int which) { > - size_t off = tcg_current_code_size(s); > - s->tb_jmp_reset_offset[which] = off; > - /* Make sure that we didn't overflow the stored offset. */ > - assert(s->tb_jmp_reset_offset[which] == off); > + /* > + * We will check for overflow at the end of the opcode loop in > + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX. > + */ > + s->tb_jmp_reset_offset[which] = tcg_current_code_size(s); > } > > #include "tcg-target.c.inc" > -- > 2.25.1
On 11/3/20 4:39 AM, Richard Henderson wrote: > The range check done here is done later, more appropriately, > at the end of tcg_gen_code. Maybe mention commit 6e6c4efed99 ("tcg: Restart after TB code generation overflow")? (no need to repost). > There, a failing range check > results in a returned error code, which causes the TB to be > restarted at half the size. > > Reported-by: Sai Pavan Boddu <saipava@xilinx.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > > Sai, would you try this against your failing testcase? > > > r~ > > --- > tcg/tcg.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index f49f1a7f35..43c6cf8f52 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s) > > static void set_jmp_reset_offset(TCGContext *s, int which) > { > - size_t off = tcg_current_code_size(s); > - s->tb_jmp_reset_offset[which] = off; > - /* Make sure that we didn't overflow the stored offset. */ > - assert(s->tb_jmp_reset_offset[which] == off); > + /* > + * We will check for overflow at the end of the opcode loop in > + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX. > + */ > + s->tb_jmp_reset_offset[which] = tcg_current_code_size(s); > } > > #include "tcg-target.c.inc" > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/tcg/tcg.c b/tcg/tcg.c index f49f1a7f35..43c6cf8f52 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s) static void set_jmp_reset_offset(TCGContext *s, int which) { - size_t off = tcg_current_code_size(s); - s->tb_jmp_reset_offset[which] = off; - /* Make sure that we didn't overflow the stored offset. */ - assert(s->tb_jmp_reset_offset[which] == off); + /* + * We will check for overflow at the end of the opcode loop in + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX. + */ + s->tb_jmp_reset_offset[which] = tcg_current_code_size(s); } #include "tcg-target.c.inc"
The range check done here is done later, more appropriately, at the end of tcg_gen_code. There, a failing range check results in a returned error code, which causes the TB to be restarted at half the size. Reported-by: Sai Pavan Boddu <saipava@xilinx.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Sai, would you try this against your failing testcase? r~ --- tcg/tcg.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.25.1