sim: add __attribute__((unused)) to <arch>_fill_argbuf_tp function

Message ID 20231214162139.8415-1-simon.marchi@efficios.com
State New
Headers
Series sim: add __attribute__((unused)) to <arch>_fill_argbuf_tp function |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Simon Marchi Dec. 14, 2023, 4:21 p.m. UTC
  I get this when building with --enable-targets=all:

      CC       m32r/mloopx.o
    m32r/mloopx.c:37:1: error: ‘m32rxf_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
       37 | m32rxf_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
          | ^~~~~~~~~~~~~~~~~~~~~

      CC       m32r/mloop2.o
    m32r/mloop2.c:37:1: error: ‘m32r2f_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
       37 | m32r2f_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
          | ^~~~~~~~~~~~~~~~~~~~~

I don't know this code, so I didn't do an in-depth analysis of the
situation.  Adding the __attribute__((unused)) makes the warning go away
(this is what this patch does), but perhaps there's a better way or the
warning is the sign that something is wrong.

Change-Id: I5ef46b3dd790d6adf7c752ad5ba9676d4a41e043
---
 sim/common/genmloop.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: f5d420bbceeb1d5065c3d91dd7c6c1e43f855ca1
  

Comments

Mike Frysinger Dec. 14, 2023, 9:39 p.m. UTC | #1
On 14 Dec 2023 11:21, Simon Marchi wrote:
> I get this when building with --enable-targets=all:
> 
>       CC       m32r/mloopx.o
>     m32r/mloopx.c:37:1: error: ‘m32rxf_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
>        37 | m32rxf_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
>           | ^~~~~~~~~~~~~~~~~~~~~
> 
>       CC       m32r/mloop2.o
>     m32r/mloop2.c:37:1: error: ‘m32r2f_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
>        37 | m32r2f_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
>           | ^~~~~~~~~~~~~~~~~~~~~
> 
> I don't know this code, so I didn't do an in-depth analysis of the
> situation.  Adding the __attribute__((unused)) makes the warning go away
> (this is what this patch does), but perhaps there's a better way or the
> warning is the sign that something is wrong.

pretty sure this is a shortcoming in the m32r mloop2.in & mloopx.in files and
we should fix it there.  the patch below should fix it.

it also seems like the loop files are not respecting fast_p, but one thing at a
time i guess.
-mike

--- a/sim/m32r/mloop2.in
+++ b/sim/m32r/mloop2.in
@@ -90,8 +90,7 @@ emit_full16 (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn, SCACHE *sc,
   @cpu@_emit_before (current_cpu, sc, pc, 1);
   id = emit_16 (current_cpu, pc, insn, sc + 1, 0, 0);
   @cpu@_emit_after (current_cpu, sc + 2, pc);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
   return id;
 }
 
@@ -140,15 +139,13 @@ emit_full_parallel (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn,
 
   /* FIXME: No need to handle this parallelly if second is nop.  */
   id = emit_16 (current_cpu, pc, insn >> 16, sc + 1, 0, 1);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
 
   @cpu@_emit_before (current_cpu, sc + 2, pc, 0);
 
   /* Note that this can never be a cti.  No cti's go in the S pipeline.  */
   id2 = emit_16 (current_cpu, pc + 2, insn & 0x7fff, sc + 3, 0, 0);
-  sc[3].argbuf.trace_p = trace_p;
-  sc[3].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[3].argbuf, trace_p, profile_p);
 
   /* Set sc/snc insns notion of where to skip to.  */
   if (IDESC_SKIP_P (id))
@@ -184,8 +181,7 @@ emit_full32 (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn, SCACHE *sc,
   @cpu@_emit_before (current_cpu, sc, pc, 1);
   id = emit_32 (current_cpu, pc, insn, sc + 1, 0);
   @cpu@_emit_after (current_cpu, sc + 2, pc);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
   return id;
 }
 
--- a/sim/m32r/mloopx.in
+++ b/sim/m32r/mloopx.in
@@ -90,8 +90,7 @@ emit_full16 (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn, SCACHE *sc,
   @cpu@_emit_before (current_cpu, sc, pc, 1);
   id = emit_16 (current_cpu, pc, insn, sc + 1, 0, 0);
   @cpu@_emit_after (current_cpu, sc + 2, pc);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
   return id;
 }
 
@@ -140,15 +139,13 @@ emit_full_parallel (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn,
 
   /* FIXME: No need to handle this parallelly if second is nop.  */
   id = emit_16 (current_cpu, pc, insn >> 16, sc + 1, 0, 1);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
 
   @cpu@_emit_before (current_cpu, sc + 2, pc, 0);
 
   /* Note that this can never be a cti.  No cti's go in the S pipeline.  */
   id2 = emit_16 (current_cpu, pc + 2, insn & 0x7fff, sc + 3, 0, 0);
-  sc[3].argbuf.trace_p = trace_p;
-  sc[3].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[3].argbuf, trace_p, profile_p);
 
   /* Set sc/snc insns notion of where to skip to.  */
   if (IDESC_SKIP_P (id))
@@ -184,8 +181,7 @@ emit_full32 (SIM_CPU *current_cpu, PCADDR pc, CGEN_INSN_INT insn, SCACHE *sc,
   @cpu@_emit_before (current_cpu, sc, pc, 1);
   id = emit_32 (current_cpu, pc, insn, sc + 1, 0);
   @cpu@_emit_after (current_cpu, sc + 2, pc);
-  sc[1].argbuf.trace_p = trace_p;
-  sc[1].argbuf.profile_p = profile_p;
+  @cpu@_fill_argbuf_tp (current_cpu, &sc[1].argbuf, trace_p, profile_p);
   return id;
 }
  
Simon Marchi Dec. 15, 2023, 2:32 a.m. UTC | #2
On 2023-12-14 16:39, Mike Frysinger wrote:
> On 14 Dec 2023 11:21, Simon Marchi wrote:
>> I get this when building with --enable-targets=all:
>>
>>       CC       m32r/mloopx.o
>>     m32r/mloopx.c:37:1: error: ‘m32rxf_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
>>        37 | m32rxf_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
>>           | ^~~~~~~~~~~~~~~~~~~~~
>>
>>       CC       m32r/mloop2.o
>>     m32r/mloop2.c:37:1: error: ‘m32r2f_fill_argbuf_tp’ defined but not used [-Werror=unused-function]
>>        37 | m32r2f_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
>>           | ^~~~~~~~~~~~~~~~~~~~~
>>
>> I don't know this code, so I didn't do an in-depth analysis of the
>> situation.  Adding the __attribute__((unused)) makes the warning go away
>> (this is what this patch does), but perhaps there's a better way or the
>> warning is the sign that something is wrong.
> 
> pretty sure this is a shortcoming in the m32r mloop2.in & mloopx.in files and
> we should fix it there.  the patch below should fix it.
> 
> it also seems like the loop files are not respecting fast_p, but one thing at a
> time i guess.
> -mike

I know nothing about that code, so I trust you on that.  I tried your
patch, and it works for me, so you can add my:

Tested-By: Simon Marchi <simon.marchi@polymtl.ca>

Btw, is it possible that the dependencies in m32r/local.mk are not
correct, for the various mloop files?  After applying your patch, I had
to do a clean first.

The rule to generate mloop2.c is:

    %D%/mloop2.c %D%/eng2.h: %D%/stamp-mloop-2 ; @true
    %D%/stamp-mloop-2: $(srccom)/genmloop.sh %D%/mloop.in

Should it depend on mloop2.in instead of mloop.in?  Likewise for mloopx.

Simon
  
Mike Frysinger Dec. 15, 2023, 3:46 a.m. UTC | #3
On 14 Dec 2023 21:32, Simon Marchi wrote:
> Btw, is it possible that the dependencies in m32r/local.mk are not
> correct, for the various mloop files?  After applying your patch, I had
> to do a clean first.
> 
> The rule to generate mloop2.c is:
> 
>     %D%/mloop2.c %D%/eng2.h: %D%/stamp-mloop-2 ; @true
>     %D%/stamp-mloop-2: $(srccom)/genmloop.sh %D%/mloop.in
> 
> Should it depend on mloop2.in instead of mloop.in?  Likewise for mloopx.

yeah, it should.  m32r is weird in that it has multiple mloop files -- other
ports that generate multiple outputs do it from a single mloop.in input.  so
looks like i missed that when migrating everything to local.mk.  i broke it
with commit 0a129eb19a773d930d60b084209570f663db2053.  pushed the fix below.
-mike

From 10802d9ac0c6e39a672f86224ec89a9a54a37b9c Mon Sep 17 00:00:00 2001
From: Mike Frysinger <vapier@gentoo.org>
Date: Thu, 14 Dec 2023 22:45:22 -0500
Subject: [PATCH] sim: m32r: fix mloop.in variant stamp deps

The migration to local.mk in commit 0a129eb19a773d930d60b084209570f663db2053
accidentally listed the deps for all mloop steps as mloop.in instead of the
various variants that m32r uses.

Reported-by: Simon Marchi <simon.marchi@polymtl.ca>
---
 sim/Makefile.in   | 4 ++--
 sim/m32r/local.mk | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sim/m32r/local.mk b/sim/m32r/local.mk
index 182d8879b434..13f6ea5f9570 100644
--- a/sim/m32r/local.mk
+++ b/sim/m32r/local.mk
@@ -105,7 +105,7 @@ BUILT_SOURCES += \
 
 ## FIXME: Use of `mono' is wip.
 %D%/mloopx.c %D%/engx.h: %D%/stamp-mloop-x ; @true
-%D%/stamp-mloop-x: $(srccom)/genmloop.sh %D%/mloop.in
+%D%/stamp-mloop-x: $(srccom)/genmloop.sh %D%/mloopx.in
 	$(AM_V_GEN)$(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
 		-mono -no-fast -pbb -parallel-write -switch semx-switch.c \
 		-cpu m32rxf \
@@ -116,7 +116,7 @@ BUILT_SOURCES += \
 
 ## FIXME: Use of `mono' is wip.
 %D%/mloop2.c %D%/eng2.h: %D%/stamp-mloop-2 ; @true
-%D%/stamp-mloop-2: $(srccom)/genmloop.sh %D%/mloop.in
+%D%/stamp-mloop-2: $(srccom)/genmloop.sh %D%/mloop2.in
 	$(AM_V_GEN)$(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
 		-mono -no-fast -pbb -parallel-write -switch sem2-switch.c \
 		-cpu m32r2f \
  

Patch

diff --git a/sim/common/genmloop.sh b/sim/common/genmloop.sh
index 4312dc5468f6..b466823331b5 100755
--- a/sim/common/genmloop.sh
+++ b/sim/common/genmloop.sh
@@ -336,7 +336,7 @@  static INLINE void
 
 /* Fill in tracing/profiling fields of an ARGBUF.  */
 
-static INLINE void
+__attribute__((unused)) static INLINE void
 @prefix@_fill_argbuf_tp (const SIM_CPU *cpu, ARGBUF *abuf,
 		       int trace_p, int profile_p)
 {