qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
@ 2011-04-13 20:38 Stefan Weil
  2011-04-13 21:05 ` Peter Maydell
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-13 20:38 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: QEMU Developers

gen_pc_load was introduced in commit
d2856f1ad4c259e5766847c49acbb4e390731bd4.
The only reason for parameter searched_pc was
a debug statement in target-i386/translate.c.

Remove searched_pc from the debug statement
and from the parameter list of gen_pc_load.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 exec-all.h                    |    2 +-
 target-alpha/translate.c      |    3 +--
 target-arm/translate.c        |    3 +--
 target-cris/translate.c       |    3 +--
 target-i386/translate.c       |    7 +++----
 target-lm32/translate.c       |    3 +--
 target-m68k/translate.c       |    3 +--
 target-microblaze/translate.c |    3 +--
 target-mips/translate.c       |    3 +--
 target-ppc/translate.c        |    3 +--
 target-s390x/translate.c      |    3 +--
 target-sh4/translate.c        |    3 +--
 target-sparc/translate.c      |    3 +--
 target-unicore32/translate.c  |    3 +--
 translate-all.c               |    2 +-
 15 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 496c001..8466f2b 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -78,7 +78,7 @@ extern uint16_t gen_opc_icount[OPC_BUF_SIZE];
 void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb);
 void gen_intermediate_code_pc(CPUState *env, struct TranslationBlock *tb);
 void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc);
+                 int pc_pos, void *puc);
 
 void cpu_gen_init(void);
 int cpu_gen_code(CPUState *env, struct TranslationBlock *tb,
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 96e922b..21f54ac 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -3367,8 +3367,7 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
     return env;
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6190028..64a4ffd 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9817,8 +9817,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 #endif
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->regs[15] = gen_opc_pc[pc_pos];
     env->condexec_bits = gen_opc_condexec_bits[pc_pos];
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 1c03fa5..9c3ebb6 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3604,8 +3604,7 @@ void cpu_reset (CPUCRISState *env)
 #endif
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
 	env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7d1340e..16d9cb7 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7890,8 +7890,7 @@ void gen_intermediate_code_pc(CPUState *env, TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     int cc_op;
 #ifdef DEBUG_DISAS
@@ -7903,8 +7902,8 @@ void gen_pc_load(CPUState *env, TranslationBlock *tb,
                 qemu_log("0x%04x: " TARGET_FMT_lx "\n", i, gen_opc_pc[i]);
             }
         }
-        qemu_log("spc=0x%08lx pc_pos=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
-                searched_pc, pc_pos, gen_opc_pc[pc_pos] - tb->cs_base,
+        qemu_log("pc_pos=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
+                pc_pos, gen_opc_pc[pc_pos] - tb->cs_base,
                 (uint32_t)tb->cs_base);
     }
 #endif
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index efc9b5a..1939024 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1212,8 +1212,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
     cpu_fprintf(f, "\n\n");
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 038c0af..7438f9a 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3113,8 +3113,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
     cpu_fprintf (f, "FPRESULT = %12g\n", *(double *)&env->fp_result);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index bff3a11..f7d6154 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1940,8 +1940,7 @@ void cpu_reset (CPUState *env)
 #endif
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->sregs[SR_PC] = gen_opc_pc[pc_pos];
 }
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 953c528..81d7c8c 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12737,8 +12737,7 @@ void cpu_reset (CPUMIPSState *env)
     env->exception_index = EXCP_NONE;
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->active_tc.PC = gen_opc_pc[pc_pos];
     env->hflags &= ~MIPS_HFLAG_BMASK;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3c3ee24..036f983 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9367,8 +9367,7 @@ void gen_intermediate_code_pc (CPUState *env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->nip = gen_opc_pc[pc_pos];
 }
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index d33bfb1..0c42cae 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -54,8 +54,7 @@ void gen_intermediate_code_pc (CPUState *env, struct TranslationBlock *tb)
 {
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->psw.addr = gen_opc_pc[pc_pos];
 }
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index 88098d7..72ad036 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -2069,8 +2069,7 @@ void gen_intermediate_code_pc(CPUState * env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->pc = gen_opc_pc[pc_pos];
     env->flags = gen_opc_hflags[pc_pos];
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 883ecd2..4d09b42 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5080,8 +5080,7 @@ void gen_intermediate_code_init(CPUSPARCState *env)
     }
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     target_ulong npc;
     env->pc = gen_opc_pc[pc_pos];
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index a6ba991..8432fc9 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -2098,8 +2098,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 #endif
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-        unsigned long searched_pc, int pc_pos, void *puc)
+void gen_pc_load(CPUState *env, TranslationBlock *tb, int pc_pos, void *puc)
 {
     env->regs[31] = gen_opc_pc[pc_pos];
 }
diff --git a/translate-all.c b/translate-all.c
index efcfb9a..f286d6c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -157,7 +157,7 @@ int cpu_restore_state(TranslationBlock *tb,
         j--;
     env->icount_decr.u16.low -= gen_opc_icount[j];
 
-    gen_pc_load(env, tb, searched_pc, j, puc);
+    gen_pc_load(env, tb, j, puc);
 
 #ifdef CONFIG_PROFILER
     s->restore_time += profile_getclock() - ti;
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-13 20:38 [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load Stefan Weil
@ 2011-04-13 21:05 ` Peter Maydell
  2011-04-14 18:50   ` Stefan Weil
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2011-04-13 21:05 UTC (permalink / raw)
  To: Stefan Weil; +Cc: QEMU Developers, Aurelien Jarno

On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
> gen_pc_load was introduced in commit
> d2856f1ad4c259e5766847c49acbb4e390731bd4.
> The only reason for parameter searched_pc was
> a debug statement in target-i386/translate.c.
>
> Remove searched_pc from the debug statement
> and from the parameter list of gen_pc_load.

No issues with the meat of the patch, but if we're going to
change all the callers and implementations of this anyway,
is there any appetite for giving it a more appropriate name?
It doesn't generate any code, it affects more than just the
pc, and it doesn't do a load...

restore_state_to_opc() ? set_env_for_opc() ?

-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-13 21:05 ` Peter Maydell
@ 2011-04-14 18:50   ` Stefan Weil
  2011-04-17 18:27     ` Aurelien Jarno
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-14 18:50 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

Am 13.04.2011 23:05, schrieb Peter Maydell:
> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
>> gen_pc_load was introduced in commit
>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
>> The only reason for parameter searched_pc was
>> a debug statement in target-i386/translate.c.
>>
>> Remove searched_pc from the debug statement
>> and from the parameter list of gen_pc_load.
>
> No issues with the meat of the patch, but if we're going to
> change all the callers and implementations of this anyway,
> is there any appetite for giving it a more appropriate name?
> It doesn't generate any code, it affects more than just the
> pc, and it doesn't do a load...
>
> restore_state_to_opc() ? set_env_for_opc() ?
>
> -- PMM


What about cpu_restore_pc()? That's not always the whole truth,
but it's always the main action done in function n.n. which currently
is called gen_pc_load.

Or cpu_restore_helper()? Helper is very generic - it always fits.

Aurelien, please feel free to choose a name which suits bests.
I don't mind if you simply patch my patch, create a new one
or tell me which name should go into a new version of the patch
so I can send it.

Kind regards,
Stefan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-14 18:50   ` Stefan Weil
@ 2011-04-17 18:27     ` Aurelien Jarno
  2011-04-17 21:07       ` Stefan Weil
  0 siblings, 1 reply; 15+ messages in thread
From: Aurelien Jarno @ 2011-04-17 18:27 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Peter Maydell, QEMU Developers

On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
> Am 13.04.2011 23:05, schrieb Peter Maydell:
> >On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
> >>gen_pc_load was introduced in commit
> >>d2856f1ad4c259e5766847c49acbb4e390731bd4.
> >>The only reason for parameter searched_pc was
> >>a debug statement in target-i386/translate.c.
> >>
> >>Remove searched_pc from the debug statement
> >>and from the parameter list of gen_pc_load.
> >
> >No issues with the meat of the patch, but if we're going to
> >change all the callers and implementations of this anyway,
> >is there any appetite for giving it a more appropriate name?
> >It doesn't generate any code, it affects more than just the
> >pc, and it doesn't do a load...
> >
> >restore_state_to_opc() ? set_env_for_opc() ?
> >
> >-- PMM
> 
> 
> What about cpu_restore_pc()? That's not always the whole truth,
> but it's always the main action done in function n.n. which currently
> is called gen_pc_load.
> 
> Or cpu_restore_helper()? Helper is very generic - it always fits.
> 
> Aurelien, please feel free to choose a name which suits bests.
> I don't mind if you simply patch my patch, create a new one
> or tell me which name should go into a new version of the patch
> so I can send it.
> 

As Peter said, the function is doing more than simply restoring the
pc. I am fine with the name he proposed, I think restore_state_to_opc()
is a bit better.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-17 18:27     ` Aurelien Jarno
@ 2011-04-17 21:07       ` Stefan Weil
  2011-04-17 21:34         ` Peter Maydell
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-17 21:07 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

Am 17.04.2011 20:27, schrieb Aurelien Jarno:
> On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
>> Am 13.04.2011 23:05, schrieb Peter Maydell:
>>> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
>>>> gen_pc_load was introduced in commit
>>>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
>>>> The only reason for parameter searched_pc was
>>>> a debug statement in target-i386/translate.c.
>>>>
>>>> Remove searched_pc from the debug statement
>>>> and from the parameter list of gen_pc_load.
>>>
>>> No issues with the meat of the patch, but if we're going to
>>> change all the callers and implementations of this anyway,
>>> is there any appetite for giving it a more appropriate name?
>>> It doesn't generate any code, it affects more than just the
>>> pc, and it doesn't do a load...
>>>
>>> restore_state_to_opc() ? set_env_for_opc() ?
>>>
>>> -- PMM
>>
>>
>> What about cpu_restore_pc()? That's not always the whole truth,
>> but it's always the main action done in function n.n. which currently
>> is called gen_pc_load.
>>
>> Or cpu_restore_helper()? Helper is very generic - it always fits.
>>
>> Aurelien, please feel free to choose a name which suits bests.
>> I don't mind if you simply patch my patch, create a new one
>> or tell me which name should go into a new version of the patch
>> so I can send it.
>>
>
> As Peter said, the function is doing more than simply restoring the
> pc. I am fine with the name he proposed, I think restore_state_to_opc()
> is a bit better.

Ok, so I'll send a new patch which also replaces gen_pc_load
by restore_state_to_op. The new function name is longer than
the old one, but it was possible to remove one more function
parameter, therefore line lengths don't increase.

avoid over

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-17 21:07       ` Stefan Weil
@ 2011-04-17 21:34         ` Peter Maydell
  2011-04-17 21:43           ` Aurelien Jarno
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2011-04-17 21:34 UTC (permalink / raw)
  To: Stefan Weil; +Cc: QEMU Developers, Aurelien Jarno

On 17 April 2011 22:07, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 17.04.2011 20:27, schrieb Aurelien Jarno:
>>
>> On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
>>>
>>> Am 13.04.2011 23:05, schrieb Peter Maydell:
>>>>
>>>> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
>>>>>
>>>>> gen_pc_load was introduced in commit
>>>>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
>>>>> The only reason for parameter searched_pc was
>>>>> a debug statement in target-i386/translate.c.
>>>>>
>>>>> Remove searched_pc from the debug statement
>>>>> and from the parameter list of gen_pc_load.
>>>>
>>>> No issues with the meat of the patch, but if we're going to
>>>> change all the callers and implementations of this anyway,
>>>> is there any appetite for giving it a more appropriate name?
>>>> It doesn't generate any code, it affects more than just the
>>>> pc, and it doesn't do a load...
>>>>
>>>> restore_state_to_opc() ? set_env_for_opc() ?
>>>>
>>>> -- PMM
>>>
>>>
>>> What about cpu_restore_pc()? That's not always the whole truth,
>>> but it's always the main action done in function n.n. which currently
>>> is called gen_pc_load.
>>>
>>> Or cpu_restore_helper()? Helper is very generic - it always fits.
>>>
>>> Aurelien, please feel free to choose a name which suits bests.
>>> I don't mind if you simply patch my patch, create a new one
>>> or tell me which name should go into a new version of the patch
>>> so I can send it.
>>>
>>
>> As Peter said, the function is doing more than simply restoring the
>> pc. I am fine with the name he proposed, I think restore_state_to_opc()
>> is a bit better.
>
> Ok, so I'll send a new patch which also replaces gen_pc_load
> by restore_state_to_op.

That's _to_opc, not _to_op : I was trying to be consistent with
the naming of the gen_opc_* arrays.

-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-17 21:34         ` Peter Maydell
@ 2011-04-17 21:43           ` Aurelien Jarno
  2011-04-18 16:35             ` Stefan Weil
  0 siblings, 1 reply; 15+ messages in thread
From: Aurelien Jarno @ 2011-04-17 21:43 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On Sun, Apr 17, 2011 at 10:34:47PM +0100, Peter Maydell wrote:
> On 17 April 2011 22:07, Stefan Weil <weil@mail.berlios.de> wrote:
> > Am 17.04.2011 20:27, schrieb Aurelien Jarno:
> >>
> >> On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
> >>>
> >>> Am 13.04.2011 23:05, schrieb Peter Maydell:
> >>>>
> >>>> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
> >>>>>
> >>>>> gen_pc_load was introduced in commit
> >>>>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
> >>>>> The only reason for parameter searched_pc was
> >>>>> a debug statement in target-i386/translate.c.
> >>>>>
> >>>>> Remove searched_pc from the debug statement
> >>>>> and from the parameter list of gen_pc_load.
> >>>>
> >>>> No issues with the meat of the patch, but if we're going to
> >>>> change all the callers and implementations of this anyway,
> >>>> is there any appetite for giving it a more appropriate name?
> >>>> It doesn't generate any code, it affects more than just the
> >>>> pc, and it doesn't do a load...
> >>>>
> >>>> restore_state_to_opc() ? set_env_for_opc() ?
> >>>>
> >>>> -- PMM
> >>>
> >>>
> >>> What about cpu_restore_pc()? That's not always the whole truth,
> >>> but it's always the main action done in function n.n. which currently
> >>> is called gen_pc_load.
> >>>
> >>> Or cpu_restore_helper()? Helper is very generic - it always fits.
> >>>
> >>> Aurelien, please feel free to choose a name which suits bests.
> >>> I don't mind if you simply patch my patch, create a new one
> >>> or tell me which name should go into a new version of the patch
> >>> so I can send it.
> >>>
> >>
> >> As Peter said, the function is doing more than simply restoring the
> >> pc. I am fine with the name he proposed, I think restore_state_to_opc()
> >> is a bit better.
> >
> > Ok, so I'll send a new patch which also replaces gen_pc_load
> > by restore_state_to_op.
> 
> That's _to_opc, not _to_op : I was trying to be consistent with
> the naming of the gen_opc_* arrays.
> 

Oops, sorry, just a cut & paste mistake.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load
  2011-04-17 21:43           ` Aurelien Jarno
@ 2011-04-18 16:35             ` Stefan Weil
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function Stefan Weil
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state Stefan Weil
  0 siblings, 2 replies; 15+ messages in thread
From: Stefan Weil @ 2011-04-18 16:35 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

Am 17.04.2011 23:43, schrieb Aurelien Jarno:
> On Sun, Apr 17, 2011 at 10:34:47PM +0100, Peter Maydell wrote:
>> On 17 April 2011 22:07, Stefan Weil <weil@mail.berlios.de> wrote:
>>> Am 17.04.2011 20:27, schrieb Aurelien Jarno:
>>>>
>>>> On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
>>>>>
>>>>> Am 13.04.2011 23:05, schrieb Peter Maydell:
>>>>>>
>>>>>> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
>>>>>>>
>>>>>>> gen_pc_load was introduced in commit
>>>>>>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
>>>>>>> The only reason for parameter searched_pc was
>>>>>>> a debug statement in target-i386/translate.c.
>>>>>>>
>>>>>>> Remove searched_pc from the debug statement
>>>>>>> and from the parameter list of gen_pc_load.
>>>>>>
>>>>>> No issues with the meat of the patch, but if we're going to
>>>>>> change all the callers and implementations of this anyway,
>>>>>> is there any appetite for giving it a more appropriate name?
>>>>>> It doesn't generate any code, it affects more than just the
>>>>>> pc, and it doesn't do a load...
>>>>>>
>>>>>> restore_state_to_opc() ? set_env_for_opc() ?
>>>>>>
>>>>>> -- PMM
>>>>>
>>>>>
>>>>> What about cpu_restore_pc()? That's not always the whole truth,
>>>>> but it's always the main action done in function n.n. which currently
>>>>> is called gen_pc_load.
>>>>>
>>>>> Or cpu_restore_helper()? Helper is very generic - it always fits.
>>>>>
>>>>> Aurelien, please feel free to choose a name which suits bests.
>>>>> I don't mind if you simply patch my patch, create a new one
>>>>> or tell me which name should go into a new version of the patch
>>>>> so I can send it.
>>>>>
>>>>
>>>> As Peter said, the function is doing more than simply restoring the
>>>> pc. I am fine with the name he proposed, I think restore_state_to_opc()
>>>> is a bit better.
>>>
>>> Ok, so I'll send a new patch which also replaces gen_pc_load
>>> by restore_state_to_op.
>>
>> That's _to_opc, not _to_op : I was trying to be consistent with
>> the naming of the gen_opc_* arrays.
>>
>
> Oops, sorry, just a cut & paste mistake.

I agree, but it was my mistake.

The first of the following patches is an improved version of my previous 
patch.
It uses the correct name restore_state_to_opc and also removes a second 
parameter.

The second patch is based on the first and removes a parameter
from another function.

[PATCH 1/2] Remove unused function parameters from gen_pc_load and 
rename the function
[PATCH 2/2] Remove unused function parameter from cpu_restore_state

Cheers,
Stefan W.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function
  2011-04-18 16:35             ` Stefan Weil
@ 2011-04-18 16:39               ` Stefan Weil
  2011-04-18 17:15                 ` Peter Maydell
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state Stefan Weil
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-18 16:39 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

Function gen_pc_load was introduced in commit
d2856f1ad4c259e5766847c49acbb4e390731bd4.
The only reason for parameter searched_pc was
a debug statement in target-i386/translate.c.

Parameter puc was needed by target-sparc until
commit d2856f1ad4c259e5766847c49acbb4e390731bd4.

Remove searched_pc from the debug statement and remove both
parameters from the parameter list of gen_pc_load.

As the function name gen_pc_load was also misleading,
it is now called restore_state_to_opc. This new name
was suggested by Peter Maydell, thanks.

v2: Remove last parameter, too, and rename the function.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 exec-all.h                    |    4 ++--
 target-alpha/translate.c      |    3 +--
 target-arm/translate.c        |    7 +++----
 target-cris/translate.c       |    3 +--
 target-i386/translate.c       |    7 +++----
 target-lm32/translate.c       |    3 +--
 target-m68k/translate.c       |    3 +--
 target-microblaze/translate.c |    3 +--
 target-mips/translate.c       |    3 +--
 target-ppc/translate.c        |    3 +--
 target-s390x/translate.c      |    3 +--
 target-sh4/translate.c        |    3 +--
 target-sparc/translate.c      |    3 +--
 target-unicore32/translate.c  |    3 +--
 translate-all.c               |    2 +-
 15 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 496c001..29fd322 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -77,8 +77,8 @@ extern uint16_t gen_opc_icount[OPC_BUF_SIZE];
 
 void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb);
 void gen_intermediate_code_pc(CPUState *env, struct TranslationBlock *tb);
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc);
+void restore_state_to_opc(CPUState *env, struct TranslationBlock *tb,
+                          int pc_pos);
 
 void cpu_gen_init(void);
 int cpu_gen_code(CPUState *env, struct TranslationBlock *tb,
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 96e922b..456ba51 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -3367,8 +3367,7 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
     return env;
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6190028..7e7652e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9551,8 +9551,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
      * This is handled in the same way as restoration of the
      * PC in these situations: we will be called again with search_pc=1
      * and generate a mapping of the condexec bits for each PC in
-     * gen_opc_condexec_bits[]. gen_pc_load[] then uses this to restore
-     * the condexec bits.
+     * gen_opc_condexec_bits[]. restore_state_to_opc[] then uses
+     * this to restore the condexec bits.
      *
      * Note that there are no instructions which can read the condexec
      * bits, and none which can write non-static values to them, so
@@ -9817,8 +9817,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 #endif
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->regs[15] = gen_opc_pc[pc_pos];
     env->condexec_bits = gen_opc_condexec_bits[pc_pos];
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 1c03fa5..e2607d6 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3604,8 +3604,7 @@ void cpu_reset (CPUCRISState *env)
 #endif
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
 	env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7d1340e..199302e 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7890,8 +7890,7 @@ void gen_intermediate_code_pc(CPUState *env, TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     int cc_op;
 #ifdef DEBUG_DISAS
@@ -7903,8 +7902,8 @@ void gen_pc_load(CPUState *env, TranslationBlock *tb,
                 qemu_log("0x%04x: " TARGET_FMT_lx "\n", i, gen_opc_pc[i]);
             }
         }
-        qemu_log("spc=0x%08lx pc_pos=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
-                searched_pc, pc_pos, gen_opc_pc[pc_pos] - tb->cs_base,
+        qemu_log("pc_pos=0x%x eip=" TARGET_FMT_lx " cs_base=%x\n",
+                pc_pos, gen_opc_pc[pc_pos] - tb->cs_base,
                 (uint32_t)tb->cs_base);
     }
 #endif
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index efc9b5a..51b4f5a 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1212,8 +1212,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
     cpu_fprintf(f, "\n\n");
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 038c0af..9e5578d 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3113,8 +3113,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
     cpu_fprintf (f, "FPRESULT = %12g\n", *(double *)&env->fp_result);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->pc = gen_opc_pc[pc_pos];
 }
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index bff3a11..b47b92e 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1940,8 +1940,7 @@ void cpu_reset (CPUState *env)
 #endif
 }
 
-void gen_pc_load(CPUState *env, struct TranslationBlock *tb,
-                 unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->sregs[SR_PC] = gen_opc_pc[pc_pos];
 }
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 953c528..4eaa826 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12737,8 +12737,7 @@ void cpu_reset (CPUMIPSState *env)
     env->exception_index = EXCP_NONE;
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->active_tc.PC = gen_opc_pc[pc_pos];
     env->hflags &= ~MIPS_HFLAG_BMASK;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3c3ee24..a943dbc 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9367,8 +9367,7 @@ void gen_intermediate_code_pc (CPUState *env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->nip = gen_opc_pc[pc_pos];
 }
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index d33bfb1..7eed32c 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -54,8 +54,7 @@ void gen_intermediate_code_pc (CPUState *env, struct TranslationBlock *tb)
 {
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->psw.addr = gen_opc_pc[pc_pos];
 }
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index 88098d7..93c8636 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -2069,8 +2069,7 @@ void gen_intermediate_code_pc(CPUState * env, struct TranslationBlock *tb)
     gen_intermediate_code_internal(env, tb, 1);
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->pc = gen_opc_pc[pc_pos];
     env->flags = gen_opc_hflags[pc_pos];
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 883ecd2..3c958b2 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5080,8 +5080,7 @@ void gen_intermediate_code_init(CPUSPARCState *env)
     }
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-                unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     target_ulong npc;
     env->pc = gen_opc_pc[pc_pos];
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index a6ba991..98eaeb3 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -2098,8 +2098,7 @@ void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 #endif
 }
 
-void gen_pc_load(CPUState *env, TranslationBlock *tb,
-        unsigned long searched_pc, int pc_pos, void *puc)
+void restore_state_to_opc(CPUState *env, TranslationBlock *tb, int pc_pos)
 {
     env->regs[31] = gen_opc_pc[pc_pos];
 }
diff --git a/translate-all.c b/translate-all.c
index efcfb9a..97668b2 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -157,7 +157,7 @@ int cpu_restore_state(TranslationBlock *tb,
         j--;
     env->icount_decr.u16.low -= gen_opc_icount[j];
 
-    gen_pc_load(env, tb, searched_pc, j, puc);
+    restore_state_to_opc(env, tb, j);
 
 #ifdef CONFIG_PROFILER
     s->restore_time += profile_getclock() - ti;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state
  2011-04-18 16:35             ` Stefan Weil
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function Stefan Weil
@ 2011-04-18 16:39               ` Stefan Weil
  2011-04-18 17:28                 ` Peter Maydell
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-18 16:39 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

The previous patch removed the need for parameter puc.
Is is now unused, so remove it.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 cpu-exec.c                    |    2 +-
 exec-all.h                    |    3 +--
 exec.c                        |    9 ++++-----
 target-alpha/op_helper.c      |    2 +-
 target-arm/op_helper.c        |    2 +-
 target-cris/op_helper.c       |    2 +-
 target-i386/op_helper.c       |    2 +-
 target-lm32/op_helper.c       |    2 +-
 target-m68k/op_helper.c       |    2 +-
 target-microblaze/op_helper.c |    2 +-
 target-mips/op_helper.c       |    4 ++--
 target-ppc/op_helper.c        |    2 +-
 target-s390x/op_helper.c      |    2 +-
 target-sh4/op_helper.c        |    2 +-
 target-sparc/op_helper.c      |    2 +-
 translate-all.c               |    3 +--
 16 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 5d6c9a8..293ae10 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -800,7 +800,7 @@ static inline int handle_cpu_signal(unsigned long pc, unsigned long address,
     if (tb) {
         /* the PC is inside the translated code. It means that we have
            a virtual CPU fault */
-        cpu_restore_state(tb, env, pc, puc);
+        cpu_restore_state(tb, env, pc);
     }
 
     /* we restore the process signal mask as the sigreturn should
diff --git a/exec-all.h b/exec-all.h
index 29fd322..7c2d29f 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -84,8 +84,7 @@ void cpu_gen_init(void);
 int cpu_gen_code(CPUState *env, struct TranslationBlock *tb,
                  int *gen_code_size_ptr);
 int cpu_restore_state(struct TranslationBlock *tb,
-                      CPUState *env, unsigned long searched_pc,
-                      void *puc);
+                      CPUState *env, unsigned long searched_pc);
 void cpu_resume_from_signal(CPUState *env1, void *puc);
 void cpu_io_recompile(CPUState *env, void *retaddr);
 TranslationBlock *tb_gen_code(CPUState *env, 
diff --git a/exec.c b/exec.c
index b1ee52a..c3dc68a 100644
--- a/exec.c
+++ b/exec.c
@@ -1070,8 +1070,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
                 restore the CPU state */
 
                 current_tb_modified = 1;
-                cpu_restore_state(current_tb, env,
-                                  env->mem_io_pc, NULL);
+                cpu_restore_state(current_tb, env, env->mem_io_pc);
                 cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
                                      &current_flags);
             }
@@ -1179,7 +1178,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
                    restore the CPU state */
 
             current_tb_modified = 1;
-            cpu_restore_state(current_tb, env, pc, puc);
+            cpu_restore_state(current_tb, env, pc);
             cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
                                  &current_flags);
         }
@@ -3266,7 +3265,7 @@ static void check_watchpoint(int offset, int len_mask, int flags)
                     cpu_abort(env, "check_watchpoint: could not find TB for "
                               "pc=%p", (void *)env->mem_io_pc);
                 }
-                cpu_restore_state(tb, env, env->mem_io_pc, NULL);
+                cpu_restore_state(tb, env, env->mem_io_pc);
                 tb_phys_invalidate(tb, -1);
                 if (wp->flags & BP_STOP_BEFORE_ACCESS) {
                     env->exception_index = EXCP_DEBUG;
@@ -4301,7 +4300,7 @@ void cpu_io_recompile(CPUState *env, void *retaddr)
                   retaddr);
     }
     n = env->icount_decr.u16.low + tb->icount;
-    cpu_restore_state(tb, env, (unsigned long)retaddr, NULL);
+    cpu_restore_state(tb, env, (unsigned long)retaddr);
     /* Calculate how many instructions had been executed before the fault
        occurred.  */
     n = n - env->icount_decr.u16.low;
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index 6c2ae20..afa3afb 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -1373,7 +1373,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (likely(tb)) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         /* Exception index and error code are already set */
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 3de2610..ee7997b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -90,7 +90,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         raise_exception(env->exception_index);
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index be9eb06..34329e2 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -77,7 +77,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
 
 		/* Evaluate flags after retranslation.  */
                 helper_top_evaluate_flags();
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 43fbd0c..ba8136c 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -4837,7 +4837,7 @@ void tlb_fill(target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         raise_exception_err(env->exception_index, env->error_code);
diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
index e84ba48..c72b1df 100644
--- a/target-lm32/op_helper.c
+++ b/target-lm32/op_helper.c
@@ -95,7 +95,7 @@ void tlb_fill(target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         cpu_loop_exit();
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 0711107..9b13bdb 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -68,7 +68,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         cpu_loop_exit();
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index 39b8ec1..9df11c6 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -60,7 +60,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         cpu_loop_exit();
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index bd16ce3..5d6de1b 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -54,7 +54,7 @@ static void do_restore_state (void *pc_ptr)
     
     tb = tb_find_pc (pc);
     if (tb) {
-        cpu_restore_state (tb, env, pc, NULL);
+        cpu_restore_state(tb, env, pc);
     }
 }
 #endif
@@ -1972,7 +1972,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (tb) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         helper_raise_exception_err(env->exception_index, env->error_code);
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 8c993a1..99ddcb5 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -3741,7 +3741,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (likely(tb)) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         helper_raise_exception_err(env->exception_index, env->error_code);
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 402df2d..be455b9 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -61,7 +61,7 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
             if (likely(tb)) {
                 /* the PC is inside the translated code. It means that we have
                    a virtual CPU fault */
-                cpu_restore_state(tb, env, pc, NULL);
+                cpu_restore_state(tb, env, pc);
             }
         }
         /* XXX */
diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index c127860..b909d18 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -32,7 +32,7 @@ static void cpu_restore_state_from_retaddr(void *retaddr)
         if (tb) {
             /* the PC is inside the translated code. It means that we have
                a virtual CPU fault */
-            cpu_restore_state(tb, env, pc, NULL);
+            cpu_restore_state(tb, env, pc);
         }
     }
 }
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 854f168..ffffb8c 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -4375,7 +4375,7 @@ static void cpu_restore_state2(void *retaddr)
         if (tb) {
             /* the PC is inside the translated code. It means that we have
                a virtual CPU fault */
-            cpu_restore_state(tb, env, pc, (void *)(long)env->cond);
+            cpu_restore_state(tb, env, pc);
         }
     }
 }
diff --git a/translate-all.c b/translate-all.c
index 97668b2..2ca190c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -112,8 +112,7 @@ int cpu_gen_code(CPUState *env, TranslationBlock *tb, int *gen_code_size_ptr)
 /* The cpu state corresponding to 'searched_pc' is restored.
  */
 int cpu_restore_state(TranslationBlock *tb,
-                      CPUState *env, unsigned long searched_pc,
-                      void *puc)
+                      CPUState *env, unsigned long searched_pc)
 {
     TCGContext *s = &tcg_ctx;
     int j;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function Stefan Weil
@ 2011-04-18 17:15                 ` Peter Maydell
  2011-04-20  9:03                   ` [Qemu-devel] [PULL] Remove unused function parameters Stefan Weil
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2011-04-18 17:15 UTC (permalink / raw)
  To: Stefan Weil; +Cc: QEMU Developers, Aurelien Jarno

On 18 April 2011 17:39, Stefan Weil <weil@mail.berlios.de> wrote:
> Function gen_pc_load was introduced in commit
> d2856f1ad4c259e5766847c49acbb4e390731bd4.
> The only reason for parameter searched_pc was
> a debug statement in target-i386/translate.c.
>
> Parameter puc was needed by target-sparc until
> commit d2856f1ad4c259e5766847c49acbb4e390731bd4.

Don't you mean d7da2a10402f1644128b66414ca8f86bdea9ae7c ?

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 6190028..7e7652e 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9551,8 +9551,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      * This is handled in the same way as restoration of the
>      * PC in these situations: we will be called again with search_pc=1
>      * and generate a mapping of the condexec bits for each PC in
> -     * gen_opc_condexec_bits[]. gen_pc_load[] then uses this to restore
> -     * the condexec bits.
> +     * gen_opc_condexec_bits[]. restore_state_to_opc[] then uses
> +     * this to restore the condexec bits.

If you have to do another round of this patch then you could make
that say "restore_state_to_opc()" (round brackets rather than square),
but it's not worth doing a fresh round just to fix an existing typo.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state
  2011-04-18 16:39               ` [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state Stefan Weil
@ 2011-04-18 17:28                 ` Peter Maydell
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2011-04-18 17:28 UTC (permalink / raw)
  To: Stefan Weil; +Cc: QEMU Developers, Aurelien Jarno

On 18 April 2011 17:39, Stefan Weil <weil@mail.berlios.de> wrote:
> The previous patch removed the need for parameter puc.
> Is is now unused, so remove it.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PULL] Remove unused function parameters
  2011-04-18 17:15                 ` Peter Maydell
@ 2011-04-20  9:03                   ` Stefan Weil
  2011-04-20 10:53                     ` Aurelien Jarno
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2011-04-20  9:03 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Peter Maydell, QEMU Developers

Hello,

I updated the first patch as suggested by Peter Maydell
(Fix [] typo, fix copy+paste error with SHA1 object name
in commit message). The rest is identical, so I don't
resend it to qemu-devel.

Cheers,
Stefan Weil


The following changes since commit 8d5192ee15bc519f83741f5e413ebba5d57a6abd:
   Alexander Graf (1):
         s390x: virtio machine storage keys

are available in the git repository at:

   git://qemu.weilnetz.de/git/qemu.git/ patches

Stefan Weil (2):
       Remove unused function parameters from gen_pc_load and rename the 
function
       Remove unused function parameter from cpu_restore_state

  cpu-exec.c                    |    2 +-
  exec-all.h                    |    7 +++----
  exec.c                        |    9 ++++-----
  target-alpha/op_helper.c      |    2 +-
  target-alpha/translate.c      |    3 +--
  target-arm/op_helper.c        |    2 +-
  target-arm/translate.c        |    7 +++----
  target-cris/op_helper.c       |    2 +-
  target-cris/translate.c       |    3 +--
  target-i386/op_helper.c       |    2 +-
  target-i386/translate.c       |    7 +++----
  target-lm32/op_helper.c       |    2 +-
  target-lm32/translate.c       |    3 +--
  target-m68k/op_helper.c       |    2 +-
  target-m68k/translate.c       |    3 +--
  target-microblaze/op_helper.c |    2 +-
  target-microblaze/translate.c |    3 +--
  target-mips/op_helper.c       |    4 ++--
  target-mips/translate.c       |    3 +--
  target-ppc/op_helper.c        |    2 +-
  target-ppc/translate.c        |    3 +--
  target-s390x/op_helper.c      |    2 +-
  target-s390x/translate.c      |    3 +--
  target-sh4/op_helper.c        |    2 +-
  target-sh4/translate.c        |    3 +--
  target-sparc/op_helper.c      |    2 +-
  target-sparc/translate.c      |    3 +--
  target-unicore32/translate.c  |    3 +--
  translate-all.c               |    5 ++---
  29 files changed, 40 insertions(+), 56 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PULL] Remove unused function parameters
  2011-04-20  9:03                   ` [Qemu-devel] [PULL] Remove unused function parameters Stefan Weil
@ 2011-04-20 10:53                     ` Aurelien Jarno
  2011-04-20 10:57                       ` Peter Maydell
  0 siblings, 1 reply; 15+ messages in thread
From: Aurelien Jarno @ 2011-04-20 10:53 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Peter Maydell, QEMU Developers

On Wed, Apr 20, 2011 at 11:03:36AM +0200, Stefan Weil wrote:
> Hello,
> 
> I updated the first patch as suggested by Peter Maydell
> (Fix [] typo, fix copy+paste error with SHA1 object name
> in commit message). The rest is identical, so I don't
> resend it to qemu-devel.

Thanks, pulled. In the future please resend the patches on the mailing
list anyway if they differ from the previous patches. This can be in the
same thread as the pull. This is quite handy when you want to work on
QEMU without Internet access.

> Cheers,
> Stefan Weil
> 
> 
> The following changes since commit 8d5192ee15bc519f83741f5e413ebba5d57a6abd:
>   Alexander Graf (1):
>         s390x: virtio machine storage keys
> 
> are available in the git repository at:
> 
>   git://qemu.weilnetz.de/git/qemu.git/ patches
> 
> Stefan Weil (2):
>       Remove unused function parameters from gen_pc_load and rename
> the function
>       Remove unused function parameter from cpu_restore_state
> 
>  cpu-exec.c                    |    2 +-
>  exec-all.h                    |    7 +++----
>  exec.c                        |    9 ++++-----
>  target-alpha/op_helper.c      |    2 +-
>  target-alpha/translate.c      |    3 +--
>  target-arm/op_helper.c        |    2 +-
>  target-arm/translate.c        |    7 +++----
>  target-cris/op_helper.c       |    2 +-
>  target-cris/translate.c       |    3 +--
>  target-i386/op_helper.c       |    2 +-
>  target-i386/translate.c       |    7 +++----
>  target-lm32/op_helper.c       |    2 +-
>  target-lm32/translate.c       |    3 +--
>  target-m68k/op_helper.c       |    2 +-
>  target-m68k/translate.c       |    3 +--
>  target-microblaze/op_helper.c |    2 +-
>  target-microblaze/translate.c |    3 +--
>  target-mips/op_helper.c       |    4 ++--
>  target-mips/translate.c       |    3 +--
>  target-ppc/op_helper.c        |    2 +-
>  target-ppc/translate.c        |    3 +--
>  target-s390x/op_helper.c      |    2 +-
>  target-s390x/translate.c      |    3 +--
>  target-sh4/op_helper.c        |    2 +-
>  target-sh4/translate.c        |    3 +--
>  target-sparc/op_helper.c      |    2 +-
>  target-sparc/translate.c      |    3 +--
>  target-unicore32/translate.c  |    3 +--
>  translate-all.c               |    5 ++---
>  29 files changed, 40 insertions(+), 56 deletions(-)
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PULL] Remove unused function parameters
  2011-04-20 10:53                     ` Aurelien Jarno
@ 2011-04-20 10:57                       ` Peter Maydell
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2011-04-20 10:57 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: QEMU Developers

On 20 April 2011 11:53, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Wed, Apr 20, 2011 at 11:03:36AM +0200, Stefan Weil wrote:
>> Hello,
>>
>> I updated the first patch as suggested by Peter Maydell
>> (Fix [] typo, fix copy+paste error with SHA1 object name
>> in commit message). The rest is identical, so I don't
>> resend it to qemu-devel.
>
> Thanks, pulled. In the future please resend the patches on the mailing
> list anyway if they differ from the previous patches. This can be in the
> same thread as the pull. This is quite handy when you want to work on
> QEMU without Internet access.

Maybe we should document "how to send a pull request" on the wiki?
I remember having to quiz Anthony on irc about the preferred format
the first time I did one...

-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-04-20 10:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-13 20:38 [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load Stefan Weil
2011-04-13 21:05 ` Peter Maydell
2011-04-14 18:50   ` Stefan Weil
2011-04-17 18:27     ` Aurelien Jarno
2011-04-17 21:07       ` Stefan Weil
2011-04-17 21:34         ` Peter Maydell
2011-04-17 21:43           ` Aurelien Jarno
2011-04-18 16:35             ` Stefan Weil
2011-04-18 16:39               ` [Qemu-devel] [PATCH 1/2] Remove unused function parameters from gen_pc_load and rename the function Stefan Weil
2011-04-18 17:15                 ` Peter Maydell
2011-04-20  9:03                   ` [Qemu-devel] [PULL] Remove unused function parameters Stefan Weil
2011-04-20 10:53                     ` Aurelien Jarno
2011-04-20 10:57                       ` Peter Maydell
2011-04-18 16:39               ` [Qemu-devel] [PATCH 2/2] Remove unused function parameter from cpu_restore_state Stefan Weil
2011-04-18 17:28                 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).