Patchwork [3/3] target-arm: Use TCG temporary leak debugging facilities

login
register
mail settings
Submitter Peter Maydell
Date Feb. 23, 2011, 3:19 p.m.
Message ID <1298474376-20495-4-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/84188/
State New
Headers show

Comments

Peter Maydell - Feb. 23, 2011, 3:19 p.m.
Use the new TCG temporary leak debugging facilities to
check that each ARM instruction does not leak temporaries.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Blue Swirl - Feb. 25, 2011, 3:32 p.m.
On Wed, Feb 23, 2011 at 5:19 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Use the new TCG temporary leak debugging facilities to
> check that each ARM instruction does not leak temporaries.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 31067d5..b96a136 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9125,6 +9125,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>
>     gen_icount_start();
>
> +    tcg_clear_temp_count();
> +
>     /* A note on handling of the condexec (IT) bits:
>      *
>      * We want to avoid the overhead of having to write the updated condexec
> @@ -9234,6 +9236,11 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>             gen_set_label(dc->condlabel);
>             dc->condjmp = 0;
>         }
> +
> +        if (tcg_check_temp_count()) {
> +            fprintf(stderr, "TCG temporary leak before %08x\n", dc->pc);
> +        }

Perhaps this check and tcg_clear_temp_count() calls should be added
instead to tb_gen_code() in exec.c, to benefit all targets at once. PC
information will not be as accurate, though.
Peter Maydell - Feb. 25, 2011, 3:48 p.m.
On 25 February 2011 15:32, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Wed, Feb 23, 2011 at 5:19 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +
>> +        if (tcg_check_temp_count()) {
>> +            fprintf(stderr, "TCG temporary leak before %08x\n", dc->pc);
>> +        }
>
> Perhaps this check and tcg_clear_temp_count() calls should be added
> instead to tb_gen_code() in exec.c, to benefit all targets at once. PC
> information will not be as accurate, though.

You'd get a pile of false positives, for instance target-arm doesn't
bother to destroy the whole-TB temporaries like cpu_F0s because there's
no need to. We're trying to check whether the translator could
unboundedly leak temporaries...

-- PMM

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 31067d5..b96a136 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9125,6 +9125,8 @@  static inline void gen_intermediate_code_internal(CPUState *env,
 
     gen_icount_start();
 
+    tcg_clear_temp_count();
+
     /* A note on handling of the condexec (IT) bits:
      *
      * We want to avoid the overhead of having to write the updated condexec
@@ -9234,6 +9236,11 @@  static inline void gen_intermediate_code_internal(CPUState *env,
             gen_set_label(dc->condlabel);
             dc->condjmp = 0;
         }
+
+        if (tcg_check_temp_count()) {
+            fprintf(stderr, "TCG temporary leak before %08x\n", dc->pc);
+        }
+
         /* Translation stops when a conditional branch is encountered.
          * Otherwise the subsequent code could get translated several times.
          * Also stop translation when a page boundary is reached.  This