Skip to contentSkip to conversation

Unused code cleanups

PATCH Applied Peter Crosthwaite 4 participants 11 messages Apr 27, 2015 – Apr 29, 2015

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

cover Unused code cleanups

Hi Peter,

Two unused code cleanups for target-arm/cpu.h

Regards,
Peter

Peter Crosthwaite (2):
arm: cpu.h: Remove unused typdefs
arm: cpu.h: Delete unused cpu_pc_from_tb()

 target-arm/cpu.h | 14 --------------
 1 file changed, 14 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/2 arm: cpu.h: Remove unused typdefs

These CP accessor function prototypes are unused. Remove them.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu.h | 5 -----
 1 file changed, 5 deletions(-)
target-arm/cpu.h +0 -5
index 083211c..7069103 100644
@@ -93,11 +93,6 @@
 #define ARM_CPU_VIRQ 2
 #define ARM_CPU_VFIQ 3
 
-typedef void ARMWriteCPFunc(void *opaque, int cp_info,
-                            int srcreg, int operand, uint32_t value);
-typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,
-                               int dstreg, int operand);
-
 struct arm_boot_info;
 
 #define NB_MMU_MODES 7
Signature · 1 line
1.9.1
Applied
#2 lore raw

Re: [Qemu-devel] [PATCH target-arm 1/2] arm: cpu.h: Remove unused typdefs

Quoted 1 line · Peter Crosthwaite

These CP accessor function prototypes are unused. Remove them.

Applied to -trivial, thank you!

/mjt

author
#3 lore raw

PATCH 2/2 arm: cpu.h: Delete unused cpu_pc_from_tb()

In reply to Peter Crosthwaite

No code uses the cpu_pc_from_tb() function. Delete.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu.h | 9 ---------
 1 file changed, 9 deletions(-)
target-arm/cpu.h +0 -9
index 7069103..c9c5d30 100644
@@ -1874,15 +1874,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
 
 #include "exec/exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUARMState *env, TranslationBlock *tb)
-{
-    if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) {
-        env->pc = tb->pc;
-    } else {
-        env->regs[15] = tb->pc;
-    }
-}
-
 enum {
     QEMU_PSCI_CONDUIT_DISABLED = 0,
     QEMU_PSCI_CONDUIT_SMC = 1,
Signature · 1 line
1.9.1
#4 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

Quoted 12 lines · Peter Crosthwaite

No code uses the cpu_pc_from_tb() function. Delete.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu.h | 9 ---------
 1 file changed, 9 deletions(-)
target-arm/cpu.h +0 -0
index 7069103..c9c5d30 100644

target-arm/cpu.h static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, · line 1877

Quoted 4 lines
target-arm/cpu.h +0 -1
@@ -1874,15 +1874,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
 
 #include "exec/exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUARMState *env, TranslationBlock *tb)

This function is also used in target-tricore/cpu.h, and is mentioned in comments in tcg/tcg.h. Should these be removed as well?

Thanks,

/mjt

#5 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

Quoted 4 lines · Michael Tokarev
Quoted 1 line · Peter Crosthwaite

No code uses the cpu_pc_from_tb() function. Delete.

This function is also used in target-tricore/cpu.h, and is mentioned

Not _used_ but defined, the same way as it is defined but not used in target-arm/cpu.h.

Quoted 1 line

in comments in tcg/tcg.h. Should these be removed as well?

Thanks,

/mjt

#6 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

In reply to Michael Tokarev
Quoted 14 lines · Michael Tokarev
Quoted 13 lines · Peter Crosthwaite

No code uses the cpu_pc_from_tb() function. Delete.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu.h | 9 ---------
 1 file changed, 9 deletions(-)
target-arm/cpu.h +0 -0
index 7069103..c9c5d30 100644
@@ -1874,15 +1874,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
Quoted 6 lines

target-arm/cpu.h static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, · line 1877

Quoted 3 lines
target-arm/cpu.h +0 -1
 #include "exec/exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUARMState *env, TranslationBlock *tb)

This function is also used in target-tricore/cpu.h, and is mentioned in comments in tcg/tcg.h. Should these be removed as well?

The tcg/tcg.h comment should be updated:
* Otherwise, we gave up on execution of this TB before it started, and
* the caller must fix up the CPU state by calling the CPU's
* synchronize_from_tb() method with the next-TB pointer we return.
* (falling back to calling the CPU's set_pc() method with tb->pc
* if no synchronize_from_tb() method exists.)

That's a bit clunky though, which suggests we should have a cpu_synchronize_from_tb() inline function in qom/cpu.h which does the

        CPUClass *cc = CPU_GET_CLASS(cpu);
        if (cc->synchronize_from_tb) {
            cc->synchronize_from_tb(cpu, tb);
        } else {
            assert(cc->set_pc);
            cc->set_pc(cpu, tb->pc);
        }

bit that cpu-exec.c currently open-codes.

-- PMM

#7 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

Quoted 18 lines · Peter Maydell

The tcg/tcg.h comment should be updated:
* Otherwise, we gave up on execution of this TB before it started, and
* the caller must fix up the CPU state by calling the CPU's
* synchronize_from_tb() method with the next-TB pointer we return.
* (falling back to calling the CPU's set_pc() method with tb->pc
* if no synchronize_from_tb() method exists.)

That's a bit clunky though, which suggests we should have a cpu_synchronize_from_tb() inline function in qom/cpu.h which does the

        CPUClass *cc = CPU_GET_CLASS(cpu);
        if (cc->synchronize_from_tb) {
            cc->synchronize_from_tb(cpu, tb);
        } else {
            assert(cc->set_pc);
            cc->set_pc(cpu, tb->pc);
        }
Quoted 1 line

bit that cpu-exec.c currently open-codes.

...except that qom/cpu.h doesn't have the definition of the TranslationBlock struct it would need to be able to do that "tb->pc".
* we can't just include exec-all.h from here or otherwise get the TranslationBlock struct definition, because it is target CPU dependent
* we can't have the common baseclass in qom/cpu.c provide an implementation of the synchronize_from_tb method which calls set_pc, because qom/cpu.c is a common-obj-y sourcefile]
which leaves us with:
* have cpu_synchronize_from_tb() take both tb and tb->pc as args, which is pretty yucky
* give up and just update the tcg.h comment as above

I think I go for "just update the comment"...

-- PMM

#8 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

Quoted 34 lines · Peter Maydell
Quoted 18 lines · Peter Maydell

The tcg/tcg.h comment should be updated:
* Otherwise, we gave up on execution of this TB before it started, and
* the caller must fix up the CPU state by calling the CPU's
* synchronize_from_tb() method with the next-TB pointer we return.
* (falling back to calling the CPU's set_pc() method with tb->pc
* if no synchronize_from_tb() method exists.)

That's a bit clunky though, which suggests we should have a cpu_synchronize_from_tb() inline function in qom/cpu.h which does the

        CPUClass *cc = CPU_GET_CLASS(cpu);
        if (cc->synchronize_from_tb) {
            cc->synchronize_from_tb(cpu, tb);
        } else {
            assert(cc->set_pc);
            cc->set_pc(cpu, tb->pc);
        }
Quoted 1 line

bit that cpu-exec.c currently open-codes.

...except that qom/cpu.h doesn't have the definition of the TranslationBlock struct it would need to be able to do that "tb->pc".
* we can't just include exec-all.h from here or otherwise get the TranslationBlock struct definition, because it is target CPU dependent
* we can't have the common baseclass in qom/cpu.c provide an implementation of the synchronize_from_tb method which calls set_pc, because qom/cpu.c is a common-obj-y sourcefile]
which leaves us with:
* have cpu_synchronize_from_tb() take both tb and tb->pc as args, which is pretty yucky
* give up and just update the tcg.h comment as above

Quoted 2 lines

I think I go for "just update the comment"...

Does that mean a respin of this patch or should the tricore fix and the comment be a follow up?

Regards,
Peter

Quoted 2 lines

-- PMM

#9 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

Quoted 7 lines · Peter Crosthwaite
Quoted 2 lines · Peter Maydell

I think I go for "just update the comment"...

Does that mean a respin of this patch or should the tricore fix and the comment be a follow up?

I have no preference either way.

-- PMM

#10 lore raw

Re: [Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

In reply to Peter Crosthwaite
Quoted 2 lines · Peter Crosthwaite

Does that mean a respin of this patch or should the tricore fix and the comment be a follow up?

Can you please resend whole thing in one change, removing single unused function from two places and updating the comment, to fit all the pieces together? The whole thing is trivial enough to split it up and the definition of "unused" in the subject will be true :)

I think that sometimes we make a bit "too trivial" patches which aren't worth a patch by their own :) and the result is unnecessary clutter in the git history.

Thanks,

/mjt