* [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
@ 2020-11-03 3:39 Richard Henderson
2020-11-03 6:54 ` Sai Pavan Boddu
2020-11-03 7:08 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 3+ messages in thread
From: Richard Henderson @ 2020-11-03 3:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Sai Pavan Boddu
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
2020-11-03 3:39 [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset Richard Henderson
@ 2020-11-03 6:54 ` Sai Pavan Boddu
2020-11-03 7:08 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 3+ messages in thread
From: Sai Pavan Boddu @ 2020-11-03 6:54 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
2020-11-03 3:39 [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset Richard Henderson
2020-11-03 6:54 ` Sai Pavan Boddu
@ 2020-11-03 7:08 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-03 7:08 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Sai Pavan Boddu
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>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-03 7:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 3:39 [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset Richard Henderson
2020-11-03 6:54 ` Sai Pavan Boddu
2020-11-03 7:08 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.