diff mbox series

[1/5] target/ppc: fixed GEN_OPCODE behavior when PPC_DUMP_CPU is set

Message ID 20210526202104.127910-2-bruno.larsen@eldorado.org.br
State New
Headers show
Series stop collection of instruction usage statistics | expand

Commit Message

Bruno Larsen (billionai) May 26, 2021, 8:21 p.m. UTC
Before this patch, when PPC_DUMP_CPU is set, oname is added to
opc_handler_t, but GEN_OPCODE* wouldn't set it unless DO_PPC_STATISTICS
was set as well.

This patch changes it so those changes would happen when PPC_DUMP_CPU is
set, but not statistics, because the latter is being removed.

Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
---
 target/ppc/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Luis Fernando Fujita Pires May 26, 2021, 9:13 p.m. UTC | #1
From: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> Before this patch, when PPC_DUMP_CPU is set, oname is added to
> opc_handler_t, but GEN_OPCODE* wouldn't set it unless DO_PPC_STATISTICS
> was set as well.
> 
> This patch changes it so those changes would happen when PPC_DUMP_CPU is
> set, but not statistics, because the latter is being removed.
> 
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> ---
>  target/ppc/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I suggest removing dump_ppc_insns() altogether and 'oname' along with it.

Now that we're moving to decodetree, dump_ppc_insns() wouldn't show all the available opcodes anyway. And the only other locations where 'oname' is being used are when registering more than one handler for the same opcode by mistake, which won't happen anymore, as any new instructions should use decodetree.

Luis
Richard Henderson May 26, 2021, 9:24 p.m. UTC | #2
On 5/26/21 2:13 PM, Luis Fernando Fujita Pires wrote:
> From: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
>> Before this patch, when PPC_DUMP_CPU is set, oname is added to
>> opc_handler_t, but GEN_OPCODE* wouldn't set it unless DO_PPC_STATISTICS
>> was set as well.
>>
>> This patch changes it so those changes would happen when PPC_DUMP_CPU is
>> set, but not statistics, because the latter is being removed.
>>
>> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
>> ---
>>   target/ppc/translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> I suggest removing dump_ppc_insns() altogether and 'oname' along with it.
> 
> Now that we're moving to decodetree, dump_ppc_insns() wouldn't show all the available opcodes anyway. And the only other locations where 'oname' is being used are when registering more than one handler for the same opcode by mistake, which won't happen anymore, as any new instructions should use decodetree.

Agreed.

r~
David Gibson May 27, 2021, 1:18 a.m. UTC | #3
On Wed, May 26, 2021 at 02:24:51PM -0700, Richard Henderson wrote:
> On 5/26/21 2:13 PM, Luis Fernando Fujita Pires wrote:
> > From: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> > > Before this patch, when PPC_DUMP_CPU is set, oname is added to
> > > opc_handler_t, but GEN_OPCODE* wouldn't set it unless DO_PPC_STATISTICS
> > > was set as well.
> > > 
> > > This patch changes it so those changes would happen when PPC_DUMP_CPU is
> > > set, but not statistics, because the latter is being removed.
> > > 
> > > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> > > ---
> > >   target/ppc/translate.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > I suggest removing dump_ppc_insns() altogether and 'oname' along with it.
> > 
> > Now that we're moving to decodetree, dump_ppc_insns() wouldn't show all the available opcodes anyway. And the only other locations where 'oname' is being used are when registering more than one handler for the same opcode by mistake, which won't happen anymore, as any new instructions should use decodetree.
> 
> Agreed.

I'll wait for a follow up doing this then.

> 
> r~
>
diff mbox series

Patch

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ea200f9637..6c0f424d81 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1345,7 +1345,7 @@  typedef struct opcode_t {
 /*****************************************************************************/
 /* PowerPC instructions table                                                */
 
-#if defined(DO_PPC_STATISTICS)
+#if defined(PPC_DUMP_CPU)
 #define GEN_OPCODE(name, op1, op2, op3, invl, _typ, _typ2)                    \
 {                                                                             \
     .opc1 = op1,                                                              \