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

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

[Simon Marchi]

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(-)

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)
 {

base-commit: f5d420bbceeb1d5065c3d91dd7c6c1e43f855ca1
-- 
2.43.0

Re: [PATCH] sim: add __attribute__((unused)) to <arch>_fill_argbuf_tp function

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 4564 bytes --]

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;
 }
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] sim: add __attribute__((unused)) to <arch>_fill_argbuf_tp function

[Simon Marchi]

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

Re: [PATCH] sim: add __attribute__((unused)) to <arch>_fill_argbuf_tp function

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

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 \
-- 
2.43.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]