* [PATCH 01/10] sim/sh: use fabs instead of abs
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:53 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable Andrew Burgess
` (9 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
The sh simulator incorrectly uses integer abs instead of the floating
point fabs on some floating point values, fixed in this commit.
---
sim/sh/interp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sim/sh/interp.c b/sim/sh/interp.c
index fb92d9f4480..38f3f945a35 100644
--- a/sim/sh/interp.c
+++ b/sim/sh/interp.c
@@ -1401,7 +1401,7 @@ fsca_s (int in, double (*f) (double))
lower = result - error;
frac = frexp (lower, &exp);
lower = ldexp (ceil (ldexp (frac, 24)), exp - 24);
- return abs (upper - result) >= abs (lower - result) ? upper : lower;
+ return fabs (upper - result) >= fabs (lower - result) ? upper : lower;
}
static float
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
2022-10-19 15:24 ` [PATCH 01/10] sim/sh: use fabs instead of abs Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:51 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases Andrew Burgess
` (8 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
The ppc simulator, in sim_create_inferior, tries to print the function
local entry_point variable before the variable is initialized.
In this commit, I defer the debug print line until the variable has
been initialized.
---
sim/ppc/sim_calls.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sim/ppc/sim_calls.c b/sim/ppc/sim_calls.c
index 729f6dcb6f3..3dcce19f628 100644
--- a/sim/ppc/sim_calls.c
+++ b/sim/ppc/sim_calls.c
@@ -161,8 +161,6 @@ sim_create_inferior (SIM_DESC sd,
char * const *envp)
{
unsigned_word entry_point;
- TRACE(trace_gdb, ("sim_create_inferior(start_address=0x%x, ...)\n",
- entry_point));
if (simulator == NULL)
error ("No program loaded");
@@ -172,6 +170,9 @@ sim_create_inferior (SIM_DESC sd,
else
entry_point = 0xfff00000; /* ??? */
+ TRACE(trace_gdb, ("sim_create_inferior(start_address=0x%x, ...)\n",
+ entry_point));
+
psim_init(simulator);
psim_stack(simulator, argv, envp);
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable
2022-10-19 15:24 ` [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable Andrew Burgess
@ 2022-10-23 13:51 ` Mike Frysinger
2022-10-24 16:05 ` Andrew Burgess
0 siblings, 1 reply; 34+ messages in thread
From: Mike Frysinger @ 2022-10-23 13:51 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 312 bytes --]
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> The ppc simulator, in sim_create_inferior, tries to print the function
> local entry_point variable before the variable is initialized.
>
> In this commit, I defer the debug print line until the variable has
> been initialized.
lgtm
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable
2022-10-23 13:51 ` Mike Frysinger
@ 2022-10-24 16:05 ` Andrew Burgess
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-24 16:05 UTC (permalink / raw)
To: Mike Frysinger; +Cc: gdb-patches
Mike Frysinger <vapier@gentoo.org> writes:
> On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
>> The ppc simulator, in sim_create_inferior, tries to print the function
>> local entry_point variable before the variable is initialized.
>>
>> In this commit, I defer the debug print line until the variable has
>> been initialized.
>
> lgtm
Pushed.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
2022-10-19 15:24 ` [PATCH 01/10] sim/sh: use fabs instead of abs Andrew Burgess
2022-10-19 15:24 ` [PATCH 02/10] sim/ppc: don't try to print an uninitialized variable Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:50 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 04/10] sim/ppc: don't pass uninitialized value to semctl for GETVAL calls Andrew Burgess
` (7 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
In the ppc simulator's do_fstat function, which provides the fstat
call for the simulator, if the fstat is going to fail then we current
write an uninitialized buffer into the inferior.
In theory, I think this is fine, we also write the error status into
the simulated target, so, given that the fstat has failed, the target
shouldn't be relying on the buffer contents.
However, writing an uninitialized buffer means we might leak simulator
private data into the simulated target, which is probably a bad
thing. Plus it probably makes life easier if something consistent,
like all zeros, is written rather than random junk, that might look
like a successful call.
So, in this commit, I clear the stat buffer to zero before writing it
into the simulated target, if the fstat call is not run (due to a bad
file descriptor).
Another option would be to just not write the buffer into the
inferior (rather than zeroing it, and writing all zeros). This would
solve the problem of copying simulator data into the target, but I
think the all zeros will make debugging things in the simulator
easier.
---
sim/ppc/emul_netbsd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sim/ppc/emul_netbsd.c b/sim/ppc/emul_netbsd.c
index 322b584a3f1..f5fa8499bde 100644
--- a/sim/ppc/emul_netbsd.c
+++ b/sim/ppc/emul_netbsd.c
@@ -888,6 +888,8 @@ do_fstat(os_emul_data *emul,
status = fdbad (fd);
if (status == 0)
status = fstat(fd, &buf);
+ else
+ memset (&buf, 0, sizeof (buf));
emul_write_status(processor, status, errno);
write_stat(stat_buf_addr, buf, processor, cia);
}
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases
2022-10-19 15:24 ` [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases Andrew Burgess
@ 2022-10-23 13:50 ` Mike Frysinger
2022-10-24 16:25 ` Andrew Burgess
0 siblings, 1 reply; 34+ messages in thread
From: Mike Frysinger @ 2022-10-23 13:50 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> --- a/sim/ppc/emul_netbsd.c
> +++ b/sim/ppc/emul_netbsd.c
> @@ -888,6 +888,8 @@ do_fstat(os_emul_data *emul,
> status = fdbad (fd);
> if (status == 0)
> status = fstat(fd, &buf);
> + else
> + memset (&buf, 0, sizeof (buf));
i don't think this is perf critical, so simpler to do:
struct statfs buf = {};
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases
2022-10-23 13:50 ` Mike Frysinger
@ 2022-10-24 16:25 ` Andrew Burgess
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-24 16:25 UTC (permalink / raw)
To: Mike Frysinger; +Cc: gdb-patches
Mike Frysinger <vapier@gentoo.org> writes:
> On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
>> --- a/sim/ppc/emul_netbsd.c
>> +++ b/sim/ppc/emul_netbsd.c
>> @@ -888,6 +888,8 @@ do_fstat(os_emul_data *emul,
>> status = fdbad (fd);
>> if (status == 0)
>> status = fstat(fd, &buf);
>> + else
>> + memset (&buf, 0, sizeof (buf));
>
> i don't think this is perf critical, so simpler to do:
> struct statfs buf = {};
Pushed with the fix you suggest.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 04/10] sim/ppc: don't pass uninitialized value to semctl for GETVAL calls
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (2 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 03/10] sim/ppc: initialize a memory buffer in all cases Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 14:01 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang Andrew Burgess
` (6 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
When calling semctl with an GETVAL action, the 'union semun' argument
is unused.
Rather than passing an uninitialized variable (as we currently do),
just pass the integer 0. This silences some compiler warnings, and is
just as correct (given the argument is ignored).
To be honest, it might be the case that the argument is not needed at
all, however, I'm a little nervous to make this change as the amount
of testing I can do of this code is limited. Every example I found
online for a semctl/GETVAL call did pass the final argument, so that's
what I've continued to do.
---
sim/ppc/hw_sem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sim/ppc/hw_sem.c b/sim/ppc/hw_sem.c
index 937e2ad6f81..78e3dda9ae6 100644
--- a/sim/ppc/hw_sem.c
+++ b/sim/ppc/hw_sem.c
@@ -148,7 +148,7 @@ hw_sem_init_data(device *me)
error("hw_sem_init_data() semget failed\n");
}
- sem->count = semctl( sem->id, 0, GETVAL, help );
+ sem->count = semctl( sem->id, 0, GETVAL, 0 );
if (sem->count == -1)
error("hw_sem_init_data() semctl -- get value failed\n");
DTRACE(sem, ("semaphore OS value (%d)\n", sem->count) );
@@ -241,7 +241,7 @@ hw_sem_io_read_buffer(device *me,
}
/* assume target is big endian */
- u32 = H2T_4(semctl( sem->id, 0, GETVAL, help ));
+ u32 = H2T_4(semctl( sem->id, 0, GETVAL, 0 ));
DTRACE(sem, ("semaphore OS value (%d)\n", u32) );
if (u32 == 0xffffffff) {
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 04/10] sim/ppc: don't pass uninitialized value to semctl for GETVAL calls
2022-10-19 15:24 ` [PATCH 04/10] sim/ppc: don't pass uninitialized value to semctl for GETVAL calls Andrew Burgess
@ 2022-10-23 14:01 ` Mike Frysinger
0 siblings, 0 replies; 34+ messages in thread
From: Mike Frysinger @ 2022-10-23 14:01 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 837 bytes --]
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> When calling semctl with an GETVAL action, the 'union semun' argument
> is unused.
>
> Rather than passing an uninitialized variable (as we currently do),
> just pass the integer 0. This silences some compiler warnings, and is
> just as correct (given the argument is ignored).
>
> To be honest, it might be the case that the argument is not needed at
> all, however, I'm a little nervous to make this change as the amount
> of testing I can do of this code is limited. Every example I found
> online for a semctl/GETVAL call did pass the final argument, so that's
> what I've continued to do.
the semctl prototype is:
int semctl(int semid, int semnum, int cmd, ...);
i would just delete the 4th arg, but if you really want to keep the 0, OK.
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (3 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 04/10] sim/ppc: don't pass uninitialized value to semctl for GETVAL calls Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:55 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 06/10] sim/aarch64: remove two unused functions Andrew Burgess
` (5 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
In the ppc simulator, clang was warning about some code like this:
busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
The warning was:
operator '?:' has lower precedence than '+'; '+' will be evaluated first
I suspect that this is not the original authors intention.
PPC_ONE_BIT_SET_P is going to be 0 or 1, so if we evaluate the '+'
first, the condition will always be non-zero, so true. The whole
expression could then be simplified to just '1', which doesn't make
much sense.
I suspect the answer the author was expecting was either 2 or 3. Why
they didn't just write:
busy_ptr->nr_writebacks = (PPC_ONE_BIT_SET_P(out_vmask)) ? 2 : 3;
I have no clue, however, to keep the structure of the code unchanged,
I've updated things to:
busy_ptr->nr_writebacks = 1 + ((PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2);
which silences the warning from clang, and is, I am guessing, what the
original author intended.
---
sim/ppc/altivec.igen | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sim/ppc/altivec.igen b/sim/ppc/altivec.igen
index 63fe95a53d5..e7962027bd6 100644
--- a/sim/ppc/altivec.igen
+++ b/sim/ppc/altivec.igen
@@ -231,7 +231,7 @@ void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
busy_ptr->vscr_busy = 1;
if (out_vmask)
- busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
+ busy_ptr->nr_writebacks = 1 + ((PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2);
if (WITH_TRACE && ppc_trace[trace_model])
model_trace_altivec_make_busy(model_ptr, vr_mask, 0);
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang
2022-10-19 15:24 ` [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang Andrew Burgess
@ 2022-10-23 13:55 ` Mike Frysinger
2022-10-24 16:26 ` Andrew Burgess
0 siblings, 1 reply; 34+ messages in thread
From: Mike Frysinger @ 2022-10-23 13:55 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> --- a/sim/ppc/altivec.igen
> +++ b/sim/ppc/altivec.igen
> @@ -231,7 +231,7 @@ void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
> busy_ptr->vscr_busy = 1;
>
> if (out_vmask)
> - busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
> + busy_ptr->nr_writebacks = 1 + ((PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2);
should drop the paren around PPC_ONE_BIT_SET_P while you're here
otherwise lgtm
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang
2022-10-23 13:55 ` Mike Frysinger
@ 2022-10-24 16:26 ` Andrew Burgess
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-24 16:26 UTC (permalink / raw)
To: Mike Frysinger; +Cc: gdb-patches
Mike Frysinger <vapier@gentoo.org> writes:
> On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
>> --- a/sim/ppc/altivec.igen
>> +++ b/sim/ppc/altivec.igen
>> @@ -231,7 +231,7 @@ void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
>> busy_ptr->vscr_busy = 1;
>>
>> if (out_vmask)
>> - busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
>> + busy_ptr->nr_writebacks = 1 + ((PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2);
>
> should drop the paren around PPC_ONE_BIT_SET_P while you're here
Pushed with the fix you suggest.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 06/10] sim/aarch64: remove two unused functions
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (4 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 05/10] sim/ppc: fix for operator precedence warning from clang Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:55 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 07/10] sim/rx: delete an unused function Andrew Burgess
` (4 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
These functions are not used. Clang warns about the unused functions,
which is then converted into an error by -Werror.
Delete the unused functions.
---
sim/aarch64/simulator.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c
index 516a7830522..5881725cefd 100644
--- a/sim/aarch64/simulator.c
+++ b/sim/aarch64/simulator.c
@@ -83,22 +83,6 @@
} \
while (0)
-/* Helper functions used by expandLogicalImmediate. */
-
-/* for i = 1, ... N result<i-1> = 1 other bits are zero */
-static inline uint64_t
-ones (int N)
-{
- return (N == 64 ? (uint64_t)-1UL : ((1UL << N) - 1));
-}
-
-/* result<0> to val<N> */
-static inline uint64_t
-pickbit (uint64_t val, int N)
-{
- return pickbits64 (val, N, N);
-}
-
static uint64_t
expand_logical_immediate (uint32_t S, uint32_t R, uint32_t N)
{
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 07/10] sim/rx: delete an unused function
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (5 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 06/10] sim/aarch64: remove two unused functions Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:54 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 08/10] sim/h8300: avoid self assignment Andrew Burgess
` (3 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
Delete a single unused function from the rx simulator. Clang warns
about the function being unused, which is converted into an error by
-Werror.
---
sim/rx/rx.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/sim/rx/rx.c b/sim/rx/rx.c
index 70b1b9729b7..7e812c16fd7 100644
--- a/sim/rx/rx.c
+++ b/sim/rx/rx.c
@@ -754,14 +754,6 @@ typedef union {
float f;
} FloatInt;
-static inline int
-float2int (float f)
-{
- FloatInt fi;
- fi.f = f;
- return fi.i;
-}
-
static inline float
int2float (int i)
{
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 08/10] sim/h8300: avoid self assignment
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (6 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 07/10] sim/rx: delete an unused function Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 13:52 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 09/10] sim/lm32: fix some missing function declaration warnings Andrew Burgess
` (2 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
There are two places in the h8300 simulator where we assign a variable
to itself. Clang gives a warning for this, which is converted into an
error by -Werror.
Silence the warning by removing the self assignments. As these
assignments were in a complex if/then/else tree, rather than try to
adjust all the conditions, I've just replaced the self assignments
with a comment and an empty statement.
---
sim/h8300/compile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sim/h8300/compile.c b/sim/h8300/compile.c
index 9be7dd565a9..5f64b4752f0 100644
--- a/sim/h8300/compile.c
+++ b/sim/h8300/compile.c
@@ -4141,7 +4141,7 @@ step_once (SIM_DESC sd, SIM_CPU *cpu)
res = GET_B_REG (code->src.reg); /* FIXME fetch? */
if (!c && (0 <= (res >> 4) && (res >> 4) <= 9) &&
!h && (0 <= (res & 0xf) && (res & 0xf) <= 9))
- res = res; /* Value added == 0. */
+ /* Nothing. */; /* Value added == 0. */
else if (!c && (0 <= (res >> 4) && (res >> 4) <= 8) &&
!h && (10 <= (res & 0xf) && (res & 0xf) <= 15))
res = res + 0x6; /* Value added == 6. */
@@ -4174,7 +4174,7 @@ step_once (SIM_DESC sd, SIM_CPU *cpu)
res = GET_B_REG (code->src.reg); /* FIXME fetch, fetch2... */
if (!c && (0 <= (res >> 4) && (res >> 4) <= 9) &&
!h && (0 <= (res & 0xf) && (res & 0xf) <= 9))
- res = res; /* Value added == 0. */
+ /* Nothing. */; /* Value added == 0. */
else if (!c && (0 <= (res >> 4) && (res >> 4) <= 8) &&
h && (6 <= (res & 0xf) && (res & 0xf) <= 15))
res = res + 0xfa; /* Value added == 0xfa. */
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 09/10] sim/lm32: fix some missing function declaration warnings
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (7 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 08/10] sim/h8300: avoid self assignment Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-23 14:13 ` Mike Frysinger
2022-10-19 15:24 ` [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Andrew Burgess
2022-10-20 18:36 ` [PATCH 00/10] Building the sim/ tree with clang Tom Tromey
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
In the lm32 simulator, I was seeing some warnings about missing
function declarations.
The lm32 simulator has a weird header structure, in order to pull in
the full cpu.h header we need to define WANT_CPU_LM32BF. This is done
in some files, but not in others. Critically, it's not done in some
files that then use functions declared in cpu.h
In this commit I added the missing #define so that the full cpu.h can
be included.
After doing this there are still a few functions that are used
undeclared, these functions appear to be missing any declarations at
all, so I've added some to cpu.h.
With this done all the warnings when compiling lm32 are resolved for
both gcc and clang, so I've removed the SIM_WERROR_CFLAGS line from
Makefile.in, this allows lm32 to build with -Werror.
---
sim/lm32/Makefile.in | 3 ---
sim/lm32/cpu.h | 11 +++++++++++
sim/lm32/dv-lm32cpu.c | 3 +++
sim/lm32/user.c | 3 +++
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/sim/lm32/Makefile.in b/sim/lm32/Makefile.in
index d827b711d45..db15bef47a7 100644
--- a/sim/lm32/Makefile.in
+++ b/sim/lm32/Makefile.in
@@ -24,9 +24,6 @@ SIM_EXTRA_DEPS = $(CGEN_INCLUDE_DEPS) $(srcdir)/../../opcodes/lm32-desc.h \
SIM_EXTRA_CLEAN = lm32-clean
-# Some modules don't build cleanly yet.
-dv-lm32cpu.o mloop.o sem.o traps.o user.o: SIM_WERROR_CFLAGS =
-
## COMMON_POST_CONFIG_FRAG
arch = lm32
diff --git a/sim/lm32/cpu.h b/sim/lm32/cpu.h
index 05b98be8cf1..d025065f2ba 100644
--- a/sim/lm32/cpu.h
+++ b/sim/lm32/cpu.h
@@ -163,6 +163,17 @@ struct scache {
struct argbuf argbuf;
};
+/* From traps.c. */
+extern USI lm32bf_b_insn (SIM_CPU * current_cpu, USI r0, USI f_r0);
+extern USI lm32bf_divu_insn (SIM_CPU * current_cpu, IADDR pc, USI r0, USI r1, USI r2);
+extern USI lm32bf_modu_insn (SIM_CPU * current_cpu, IADDR pc, USI r0, USI r1, USI r2);
+extern void lm32bf_wcsr_insn (SIM_CPU * current_cpu, USI f_csr, USI r1);
+extern USI lm32bf_break_insn (SIM_CPU * current_cpu, IADDR pc);
+extern USI lm32bf_scall_insn (SIM_CPU * current_cpu, IADDR pc);
+
+/* From user.c. */
+extern UINT lm32bf_user_insn (SIM_CPU * current_cpu, INT r0, INT r1, UINT imm);
+
/* Macros to simplify extraction, reading and semantic code.
These define and assign the local vars that contain the insn's fields. */
diff --git a/sim/lm32/dv-lm32cpu.c b/sim/lm32/dv-lm32cpu.c
index b97580e80a3..15a08eee815 100644
--- a/sim/lm32/dv-lm32cpu.c
+++ b/sim/lm32/dv-lm32cpu.c
@@ -18,6 +18,9 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
+#define WANT_CPU lm32bf
+#define WANT_CPU_LM32BF
+
/* This must come before any other includes. */
#include "defs.h"
diff --git a/sim/lm32/user.c b/sim/lm32/user.c
index 3cc21a208ee..d301d482c1b 100644
--- a/sim/lm32/user.c
+++ b/sim/lm32/user.c
@@ -21,6 +21,9 @@
/* This must come before any other includes. */
#include "defs.h"
+#define WANT_CPU lm32bf
+#define WANT_CPU_LM32BF
+
#include "sim-main.h"
/* Handle user defined instructions. */
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (8 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 09/10] sim/lm32: fix some missing function declaration warnings Andrew Burgess
@ 2022-10-19 15:24 ` Andrew Burgess
2022-10-20 3:53 ` Tsukasa OI
` (2 more replies)
2022-10-20 18:36 ` [PATCH 00/10] Building the sim/ tree with clang Tom Tromey
10 siblings, 3 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-19 15:24 UTC (permalink / raw)
To: gdb-patches
When building the cris, m32c, and sh simulators with Clang I am seeing
build warnings from a few objects. These three simulators currently
build with -Werror, and so these warnings cause the build to fail.
When built with gcc I don't see any warnings from these targets, so
the -Werror is fine.
As the warnings are not new, in this commit, I propose that we disable
the use of -Werror for these three simulators. With this done it is
now possible to build the complete simulator tree using clang.
---
sim/cris/Makefile.in | 3 +++
sim/m32c/Makefile.in | 3 +++
sim/sh/Makefile.in | 3 +++
3 files changed, 9 insertions(+)
diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in
index d58aeee9363..53e485dca02 100644
--- a/sim/cris/Makefile.in
+++ b/sim/cris/Makefile.in
@@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \
SIM_EXTRA_CLEAN = cris-clean
+# Some modules don't build cleanly yet.
+mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS =
+
## COMMON_POST_CONFIG_FRAG
arch = cris
diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in
index 2436eb940f4..dd9b3aaf175 100644
--- a/sim/m32c/Makefile.in
+++ b/sim/m32c/Makefile.in
@@ -40,4 +40,7 @@ SIM_OBJS = \
trace.o \
$(ENDLIST)
+# Some modules don't build cleanly yet.
+mem.o: SIM_WERROR_CFLAGS =
+
## COMMON_POST_CONFIG_FRAG
diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in
index fc794f30687..a496095e767 100644
--- a/sim/sh/Makefile.in
+++ b/sim/sh/Makefile.in
@@ -24,4 +24,7 @@ SIM_OBJS = \
SIM_EXTRA_LIBS = -lm
SIM_EXTRA_DEPS = table.c code.c ppi.c
+# Some modules don't build cleanly yet.
+interp.o: SIM_WERROR_CFLAGS =
+
## COMMON_POST_CONFIG_FRAG
--
2.25.4
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-19 15:24 ` [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Andrew Burgess
@ 2022-10-20 3:53 ` Tsukasa OI
2022-10-21 15:58 ` Andrew Burgess
2022-10-20 18:36 ` Tom Tromey
2022-10-23 14:17 ` Mike Frysinger
2 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-10-20 3:53 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Hi Andrew,
It's surprising for me that you are working on that!
Based on your branch, I applied one patch (from my upcoming patchset)
each time I met an error (until I see no errors) and here's some results
(including simple patches to CGEN-generated files):
Required additional patches (from my patchset) to Andrew tree:
1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris)
2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris)
3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris)
4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris)
c.f.
1.
<https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1>
2.
<https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2>
3.
<https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3>
4.
<https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4>
Environment:
- Ubuntu 22.04.1 LTS
- LLVM + Clang 15.0.0 (built from git source; primary)
- LLVM + Clang 15.0.3 (built from git source; secondary)
And... my tree (alone with my changes) failed on Clang 15.0.0 but
succeeded on Clang 15.0.1. That is because
-Wimplicit-function-declaration was default-error on 15.0.0 but
downgraded to default-warning on 15.0.1. All error messages generated
by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a
reference.
c.f.
<https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213>
Thanks to your PATCH 09/10, I could build my working tree with Clang
except M32R simulator with Clang 15.0.0. Not only that, I could figure
out how to make M32R simulator (sort of) work. For CRIS and SuperH, I
can provide cleaner solution without disabling -Werror (at least on my
environment).
My Current Tree (to be submitted as a RFC PATCH) is available at:
<https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1>
My initial plan was to split it to smaller patchsets and submit each
ones periodically but... it seems submitting all at once seems syncing /
reviewing my and your work easier. I'll submit a RFC patchset
consisting of 40 or 41 patches in total.
Note that:
1. There are three cpu/* changes (must be reviewed on Binutils side)
2. There are some optional changes (suppresses non-error warnings)
3. PATCH 01 is a duplicate of another patchset (1 patch in total)
4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and
5. There are other patches that are pretty much the same as Andrew's.
They are coincidence because I and Andrew are trying
to fix the same issue.
Thanks,
Tsukasa
On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote:
> When building the cris, m32c, and sh simulators with Clang I am seeing
> build warnings from a few objects. These three simulators currently
> build with -Werror, and so these warnings cause the build to fail.
>
> When built with gcc I don't see any warnings from these targets, so
> the -Werror is fine.
>
> As the warnings are not new, in this commit, I propose that we disable
> the use of -Werror for these three simulators. With this done it is
> now possible to build the complete simulator tree using clang.
> ---
> sim/cris/Makefile.in | 3 +++
> sim/m32c/Makefile.in | 3 +++
> sim/sh/Makefile.in | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in
> index d58aeee9363..53e485dca02 100644
> --- a/sim/cris/Makefile.in
> +++ b/sim/cris/Makefile.in
> @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \
>
> SIM_EXTRA_CLEAN = cris-clean
>
> +# Some modules don't build cleanly yet.
> +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS =
> +
> ## COMMON_POST_CONFIG_FRAG
>
> arch = cris
> diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in
> index 2436eb940f4..dd9b3aaf175 100644
> --- a/sim/m32c/Makefile.in
> +++ b/sim/m32c/Makefile.in
> @@ -40,4 +40,7 @@ SIM_OBJS = \
> trace.o \
> $(ENDLIST)
>
> +# Some modules don't build cleanly yet.
> +mem.o: SIM_WERROR_CFLAGS =
> +
> ## COMMON_POST_CONFIG_FRAG
> diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in
> index fc794f30687..a496095e767 100644
> --- a/sim/sh/Makefile.in
> +++ b/sim/sh/Makefile.in
> @@ -24,4 +24,7 @@ SIM_OBJS = \
> SIM_EXTRA_LIBS = -lm
> SIM_EXTRA_DEPS = table.c code.c ppi.c
>
> +# Some modules don't build cleanly yet.
> +interp.o: SIM_WERROR_CFLAGS =
> +
> ## COMMON_POST_CONFIG_FRAG
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-20 3:53 ` Tsukasa OI
@ 2022-10-21 15:58 ` Andrew Burgess
2022-10-24 8:58 ` Tsukasa OI
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-21 15:58 UTC (permalink / raw)
To: Tsukasa OI, gdb-patches
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> Hi Andrew,
>
> It's surprising for me that you are working on that!
While I was reviewing your previous clang patches I tried to build the
sim/ tree with clang, but even with your patches I was hitting a bunch
of warnings/errors.
I created a set of changes as a fixup so that I could test your patches.
That is, this started as the set of extra fixes I needed on top of your
work to get the sim/ tree building with clang (for me).
Once your work was merged I decided, having done this work, I might as
well split the fixes into separate patches and get them merged. I may
have expanded slightly by looking at any warnings emitted by gcc as well
as any other non-fatal warnings from clang, and fixing anything that
looked easy.
>
> Based on your branch, I applied one patch (from my upcoming patchset)
> each time I met an error (until I see no errors) and here's some results
> (including simple patches to CGEN-generated files):
>
> Required additional patches (from my patchset) to Andrew tree:
> 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris)
> 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris)
> 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris)
> 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris)
Yeah, my version of clang is somewhat older than that (9.x !)
> c.f.
> 1.
> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1>
> 2.
> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2>
> 3.
> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3>
> 4.
> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4>
>
> Environment:
> - Ubuntu 22.04.1 LTS
> - LLVM + Clang 15.0.0 (built from git source; primary)
> - LLVM + Clang 15.0.3 (built from git source; secondary)
>
> And... my tree (alone with my changes) failed on Clang 15.0.0 but
> succeeded on Clang 15.0.1. That is because
> -Wimplicit-function-declaration was default-error on 15.0.0 but
> downgraded to default-warning on 15.0.1. All error messages generated
> by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a
> reference.
>
> c.f.
> <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213>
>
> Thanks to your PATCH 09/10, I could build my working tree with Clang
> except M32R simulator with Clang 15.0.0. Not only that, I could figure
> out how to make M32R simulator (sort of) work. For CRIS and SuperH, I
> can provide cleaner solution without disabling -Werror (at least on my
> environment).
Yes please. The disable -Werror isn't a final resting place, its more
just a stopping point so we can have something that builds - though I
think what you're saying above is that with later versions of clang
things still don't build, which is a shame.
>
> My Current Tree (to be submitted as a RFC PATCH) is available at:
> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1>
>
> My initial plan was to split it to smaller patchsets and submit each
> ones periodically but... it seems submitting all at once seems syncing /
> reviewing my and your work easier. I'll submit a RFC patchset
> consisting of 40 or 41 patches in total.
It's not clear to me if any of my patches above conflict significantly
with patches you have.
If there are any of the above that are a problem, then just let me know
and I wont merge them.
I don't have any plans to do anything more on this in the immediate
future, like I said, with the tools I have to hand right now the sim
tree now builds fine.
I have added revisiting cris/m32c/sh to my todo list, but I doubt that
will get looked at before 2023, and it sounds like you might have a
solution before then - which would be nice :)
Ideally, I'd like to push any of the above patches that don't actively
make your life harder, I'll drop anything that is a problem for you.
And then look forward to your incoming fixes.
Let me know,
Thanks,
Andrew
>
> Note that:
>
> 1. There are three cpu/* changes (must be reviewed on Binutils side)
> 2. There are some optional changes (suppresses non-error warnings)
> 3. PATCH 01 is a duplicate of another patchset (1 patch in total)
> 4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and
> 5. There are other patches that are pretty much the same as Andrew's.
> They are coincidence because I and Andrew are trying
> to fix the same issue.
>
> Thanks,
> Tsukasa
>
> On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote:
>> When building the cris, m32c, and sh simulators with Clang I am seeing
>> build warnings from a few objects. These three simulators currently
>> build with -Werror, and so these warnings cause the build to fail.
>>
>> When built with gcc I don't see any warnings from these targets, so
>> the -Werror is fine.
>>
>> As the warnings are not new, in this commit, I propose that we disable
>> the use of -Werror for these three simulators. With this done it is
>> now possible to build the complete simulator tree using clang.
>> ---
>> sim/cris/Makefile.in | 3 +++
>> sim/m32c/Makefile.in | 3 +++
>> sim/sh/Makefile.in | 3 +++
>> 3 files changed, 9 insertions(+)
>>
>> diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in
>> index d58aeee9363..53e485dca02 100644
>> --- a/sim/cris/Makefile.in
>> +++ b/sim/cris/Makefile.in
>> @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \
>>
>> SIM_EXTRA_CLEAN = cris-clean
>>
>> +# Some modules don't build cleanly yet.
>> +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS =
>> +
>> ## COMMON_POST_CONFIG_FRAG
>>
>> arch = cris
>> diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in
>> index 2436eb940f4..dd9b3aaf175 100644
>> --- a/sim/m32c/Makefile.in
>> +++ b/sim/m32c/Makefile.in
>> @@ -40,4 +40,7 @@ SIM_OBJS = \
>> trace.o \
>> $(ENDLIST)
>>
>> +# Some modules don't build cleanly yet.
>> +mem.o: SIM_WERROR_CFLAGS =
>> +
>> ## COMMON_POST_CONFIG_FRAG
>> diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in
>> index fc794f30687..a496095e767 100644
>> --- a/sim/sh/Makefile.in
>> +++ b/sim/sh/Makefile.in
>> @@ -24,4 +24,7 @@ SIM_OBJS = \
>> SIM_EXTRA_LIBS = -lm
>> SIM_EXTRA_DEPS = table.c code.c ppi.c
>>
>> +# Some modules don't build cleanly yet.
>> +interp.o: SIM_WERROR_CFLAGS =
>> +
>> ## COMMON_POST_CONFIG_FRAG
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-21 15:58 ` Andrew Burgess
@ 2022-10-24 8:58 ` Tsukasa OI
2022-10-24 16:23 ` Andrew Burgess
0 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-10-24 8:58 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 2022/10/22 0:58, Andrew Burgess wrote:
> Tsukasa OI <research_trasio@irq.a4lg.com> writes:
>
>> Hi Andrew,
>>
>> It's surprising for me that you are working on that!
>
> While I was reviewing your previous clang patches I tried to build the
> sim/ tree with clang, but even with your patches I was hitting a bunch
> of warnings/errors.
>
> I created a set of changes as a fixup so that I could test your patches.
> That is, this started as the set of extra fixes I needed on top of your
> work to get the sim/ tree building with clang (for me).
>
> Once your work was merged I decided, having done this work, I might as
> well split the fixes into separate patches and get them merged. I may
> have expanded slightly by looking at any warnings emitted by gcc as well
> as any other non-fatal warnings from clang, and fixing anything that
> looked easy.
>
>>
>> Based on your branch, I applied one patch (from my upcoming patchset)
>> each time I met an error (until I see no errors) and here's some results
>> (including simple patches to CGEN-generated files):
>>
>> Required additional patches (from my patchset) to Andrew tree:
>> 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris)
>> 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris)
>> 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris)
>> 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris)
>
> Yeah, my version of clang is somewhat older than that (9.x !)
>
>> c.f.
>> 1.
>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1>
>> 2.
>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2>
>> 3.
>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3>
>> 4.
>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4>
>>
>> Environment:
>> - Ubuntu 22.04.1 LTS
>> - LLVM + Clang 15.0.0 (built from git source; primary)
>> - LLVM + Clang 15.0.3 (built from git source; secondary)
>>
>> And... my tree (alone with my changes) failed on Clang 15.0.0 but
>> succeeded on Clang 15.0.1. That is because
>> -Wimplicit-function-declaration was default-error on 15.0.0 but
>> downgraded to default-warning on 15.0.1. All error messages generated
>> by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a
>> reference.
>>
>> c.f.
>> <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213>
>>
>> Thanks to your PATCH 09/10, I could build my working tree with Clang
>> except M32R simulator with Clang 15.0.0. Not only that, I could figure
>> out how to make M32R simulator (sort of) work. For CRIS and SuperH, I
>> can provide cleaner solution without disabling -Werror (at least on my
>> environment).
>
> Yes please. The disable -Werror isn't a final resting place, its more
> just a stopping point so we can have something that builds - though I
> think what you're saying above is that with later versions of clang
> things still don't build, which is a shame.
>
>>
>> My Current Tree (to be submitted as a RFC PATCH) is available at:
>> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1>
>>
>> My initial plan was to split it to smaller patchsets and submit each
>> ones periodically but... it seems submitting all at once seems syncing /
>> reviewing my and your work easier. I'll submit a RFC patchset
>> consisting of 40 or 41 patches in total.
>
> It's not clear to me if any of my patches above conflict significantly
> with patches you have.
>
> If there are any of the above that are a problem, then just let me know
> and I wont merge them.
>
> I don't have any plans to do anything more on this in the immediate
> future, like I said, with the tools I have to hand right now the sim
> tree now builds fine.
>
> I have added revisiting cris/m32c/sh to my todo list, but I doubt that
> will get looked at before 2023, and it sounds like you might have a
> solution before then - which would be nice :)
>
> Ideally, I'd like to push any of the above patches that don't actively
> make your life harder, I'll drop anything that is a problem for you.
> And then look forward to your incoming fixes.
>
> Let me know,
>
> Thanks,
> Andrew
c.f.: my related patchset
<https://sourceware.org/pipermail/gdb-patches/2022-October/192853.html>
Well... I just submitted my draft RFC PATCH (misnamed [PATCH] though) to
sync and discuss with your current/future work. I knew it will take
some time (**did** take some time) to investigate Clang issues and I
don't want I and you to avoid redoing each other's work.
I almost support your patchset (as I describe below).
TL;DR:
I support merging your ten patches except PATCH 04,07,10/10. Although I
personally don't like to write a patch like PATCH 10/10, it might be a
good short-term solution for now (I don't object like PATCH 04,07/10).
Each review to your patchset is as follows:
[+] : Positive
[-] : Negative
PATCH 01/10: [+]
Corresponds to my PATCH 36/40. I'll withdraw my one.
PATCH 02/10: [+]
Corresponds to my PATCH 30/40. I'll withdraw my one.
PATCH 03/10: [+]
Corresponds to my PATCH 28/40. Although my PATCH 28/40 is a bit
simpler, your patch is easy to understand and I feel this is OK
to me.
PATCH 04/10: [-]
Corresponds to my PATCH 31/40. I prefer my solution (to
initialize help with {0}).
PATCH 05/10: [+]
Corresponds (but very different) to my PATCH 27/40.
I chose to keep the semantics as in the original source code but
it seems it keeps the bug in this place. Andrew's fixes that bug
(I think) and I prefer Andrew's one.
PATCH 06/10: [+]
Corresponds to my PATCH 03/40. I'll withdraw my one.
PATCH 07/10: [-]
Corresponds to my PATCH 34/40. Although this function is currently
unused, I prefer to keep this function (with ATTRIBUTE_UNUSED)
for now.
PATCH 08/10: [+]
Corresponds to my PATCH 15/40. I chose to add dummy addition
(+ 0x0) and you chose just remove the self-assignments.
Both looks equally good and it's okay to withdraw my one.
PATCH 09/10: [+]
Copied to my working branch (as PATCH 16/40).
Needless to say, this PATCH 16/40 is yours.
PATCH 10/10: [neutral]
Personally I don't like this kind of resolution but there's no
strong reason to object to this.
I received quite a lot of review for bigger batch 1 and will submit v2
after resolving some of this (and removing changes corresponding to your
patches except PATCH 04,07/10).
Thanks,
Tsukasa
>
>>
>> Note that:
>>
>> 1. There are three cpu/* changes (must be reviewed on Binutils side)
>> 2. There are some optional changes (suppresses non-error warnings)
>> 3. PATCH 01 is a duplicate of another patchset (1 patch in total)
>> 4. PATCH 16 is entirely authored by Andrew (PATCH 09/10) and
>> 5. There are other patches that are pretty much the same as Andrew's.
>> They are coincidence because I and Andrew are trying
>> to fix the same issue.
>>
>> Thanks,
>> Tsukasa
>>
>> On 2022/10/20 0:24, Andrew Burgess via Gdb-patches wrote:
>>> When building the cris, m32c, and sh simulators with Clang I am seeing
>>> build warnings from a few objects. These three simulators currently
>>> build with -Werror, and so these warnings cause the build to fail.
>>>
>>> When built with gcc I don't see any warnings from these targets, so
>>> the -Werror is fine.
>>>
>>> As the warnings are not new, in this commit, I propose that we disable
>>> the use of -Werror for these three simulators. With this done it is
>>> now possible to build the complete simulator tree using clang.
>>> ---
>>> sim/cris/Makefile.in | 3 +++
>>> sim/m32c/Makefile.in | 3 +++
>>> sim/sh/Makefile.in | 3 +++
>>> 3 files changed, 9 insertions(+)
>>>
>>> diff --git a/sim/cris/Makefile.in b/sim/cris/Makefile.in
>>> index d58aeee9363..53e485dca02 100644
>>> --- a/sim/cris/Makefile.in
>>> +++ b/sim/cris/Makefile.in
>>> @@ -41,6 +41,9 @@ SIM_EXTRA_DEPS = \
>>>
>>> SIM_EXTRA_CLEAN = cris-clean
>>>
>>> +# Some modules don't build cleanly yet.
>>> +mloopv10f.o mloopv32f.o: SIM_WERROR_CFLAGS =
>>> +
>>> ## COMMON_POST_CONFIG_FRAG
>>>
>>> arch = cris
>>> diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in
>>> index 2436eb940f4..dd9b3aaf175 100644
>>> --- a/sim/m32c/Makefile.in
>>> +++ b/sim/m32c/Makefile.in
>>> @@ -40,4 +40,7 @@ SIM_OBJS = \
>>> trace.o \
>>> $(ENDLIST)
>>>
>>> +# Some modules don't build cleanly yet.
>>> +mem.o: SIM_WERROR_CFLAGS =
>>> +
>>> ## COMMON_POST_CONFIG_FRAG
>>> diff --git a/sim/sh/Makefile.in b/sim/sh/Makefile.in
>>> index fc794f30687..a496095e767 100644
>>> --- a/sim/sh/Makefile.in
>>> +++ b/sim/sh/Makefile.in
>>> @@ -24,4 +24,7 @@ SIM_OBJS = \
>>> SIM_EXTRA_LIBS = -lm
>>> SIM_EXTRA_DEPS = table.c code.c ppi.c
>>>
>>> +# Some modules don't build cleanly yet.
>>> +interp.o: SIM_WERROR_CFLAGS =
>>> +
>>> ## COMMON_POST_CONFIG_FRAG
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-24 8:58 ` Tsukasa OI
@ 2022-10-24 16:23 ` Andrew Burgess
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-24 16:23 UTC (permalink / raw)
To: Tsukasa OI, gdb-patches
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> On 2022/10/22 0:58, Andrew Burgess wrote:
>> Tsukasa OI <research_trasio@irq.a4lg.com> writes:
>>
>>> Hi Andrew,
>>>
>>> It's surprising for me that you are working on that!
>>
>> While I was reviewing your previous clang patches I tried to build the
>> sim/ tree with clang, but even with your patches I was hitting a bunch
>> of warnings/errors.
>>
>> I created a set of changes as a fixup so that I could test your patches.
>> That is, this started as the set of extra fixes I needed on top of your
>> work to get the sim/ tree building with clang (for me).
>>
>> Once your work was merged I decided, having done this work, I might as
>> well split the fixes into separate patches and get them merged. I may
>> have expanded slightly by looking at any warnings emitted by gcc as well
>> as any other non-fatal warnings from clang, and fixing anything that
>> looked easy.
>>
>>>
>>> Based on your branch, I applied one patch (from my upcoming patchset)
>>> each time I met an error (until I see no errors) and here's some results
>>> (including simple patches to CGEN-generated files):
>>>
>>> Required additional patches (from my patchset) to Andrew tree:
>>> 1. With PATCH 10/10 - Clang 15.0.3 : 16 (+2 for cpu/cris)
>>> 2. With PATCH 10/10 - Clang 15.0.0 : 19 (+2 for cpu/cris)
>>> 3. W/O PATCH 10/10 - Clang 15.0.3 : 20 (+2 for cpu/cris)
>>> 4. W/O PATCH 10/10 - Clang 15.0.0 : 23 (+2 for cpu/cris)
>>
>> Yeah, my version of clang is somewhat older than that (9.x !)
>>
>>> c.f.
>>> 1.
>>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-1>
>>> 2.
>>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-2>
>>> 3.
>>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-3>
>>> 4.
>>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-10-20/andrew-tree-plus-config-4>
>>>
>>> Environment:
>>> - Ubuntu 22.04.1 LTS
>>> - LLVM + Clang 15.0.0 (built from git source; primary)
>>> - LLVM + Clang 15.0.3 (built from git source; secondary)
>>>
>>> And... my tree (alone with my changes) failed on Clang 15.0.0 but
>>> succeeded on Clang 15.0.1. That is because
>>> -Wimplicit-function-declaration was default-error on 15.0.0 but
>>> downgraded to default-warning on 15.0.1. All error messages generated
>>> by Clang 15.0.0 makes sense so I'll continue using Clang 15.0.0 as a
>>> reference.
>>>
>>> c.f.
>>> <https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213>
>>>
>>> Thanks to your PATCH 09/10, I could build my working tree with Clang
>>> except M32R simulator with Clang 15.0.0. Not only that, I could figure
>>> out how to make M32R simulator (sort of) work. For CRIS and SuperH, I
>>> can provide cleaner solution without disabling -Werror (at least on my
>>> environment).
>>
>> Yes please. The disable -Werror isn't a final resting place, its more
>> just a stopping point so we can have something that builds - though I
>> think what you're saying above is that with later versions of clang
>> things still don't build, which is a shame.
>>
>>>
>>> My Current Tree (to be submitted as a RFC PATCH) is available at:
>>> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-1>
>>>
>>> My initial plan was to split it to smaller patchsets and submit each
>>> ones periodically but... it seems submitting all at once seems syncing /
>>> reviewing my and your work easier. I'll submit a RFC patchset
>>> consisting of 40 or 41 patches in total.
>>
>> It's not clear to me if any of my patches above conflict significantly
>> with patches you have.
>>
>> If there are any of the above that are a problem, then just let me know
>> and I wont merge them.
>>
>> I don't have any plans to do anything more on this in the immediate
>> future, like I said, with the tools I have to hand right now the sim
>> tree now builds fine.
>>
>> I have added revisiting cris/m32c/sh to my todo list, but I doubt that
>> will get looked at before 2023, and it sounds like you might have a
>> solution before then - which would be nice :)
>>
>> Ideally, I'd like to push any of the above patches that don't actively
>> make your life harder, I'll drop anything that is a problem for you.
>> And then look forward to your incoming fixes.
>>
>> Let me know,
>>
>> Thanks,
>> Andrew
>
> c.f.: my related patchset
> <https://sourceware.org/pipermail/gdb-patches/2022-October/192853.html>
>
>
> Well... I just submitted my draft RFC PATCH (misnamed [PATCH] though) to
> sync and discuss with your current/future work. I knew it will take
> some time (**did** take some time) to investigate Clang issues and I
> don't want I and you to avoid redoing each other's work.
>
> I almost support your patchset (as I describe below).
>
> TL;DR:
> I support merging your ten patches except PATCH 04,07,10/10. Although I
> personally don't like to write a patch like PATCH 10/10, it might be a
> good short-term solution for now (I don't object like PATCH 04,07/10).
>
> Each review to your patchset is as follows:
> [+] : Positive
> [-] : Negative
>
> PATCH 01/10: [+]
> Corresponds to my PATCH 36/40. I'll withdraw my one.
> PATCH 02/10: [+]
> Corresponds to my PATCH 30/40. I'll withdraw my one.
> PATCH 03/10: [+]
> Corresponds to my PATCH 28/40. Although my PATCH 28/40 is a bit
> simpler, your patch is easy to understand and I feel this is OK
> to me.
> PATCH 04/10: [-]
> Corresponds to my PATCH 31/40. I prefer my solution (to
> initialize help with {0}).
> PATCH 05/10: [+]
> Corresponds (but very different) to my PATCH 27/40.
> I chose to keep the semantics as in the original source code but
> it seems it keeps the bug in this place. Andrew's fixes that bug
> (I think) and I prefer Andrew's one.
> PATCH 06/10: [+]
> Corresponds to my PATCH 03/40. I'll withdraw my one.
> PATCH 07/10: [-]
> Corresponds to my PATCH 34/40. Although this function is currently
> unused, I prefer to keep this function (with ATTRIBUTE_UNUSED)
> for now.
We don't generally keep dead code around, it's always possible to pull
things back from git if needed. Before I push patch #7 I thought I'd
ask why you wanted to keep this, but were OK with patch #6.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-19 15:24 ` [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Andrew Burgess
2022-10-20 3:53 ` Tsukasa OI
@ 2022-10-20 18:36 ` Tom Tromey
2022-10-23 14:17 ` Mike Frysinger
2 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2022-10-20 18:36 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> As the warnings are not new, in this commit, I propose that we disable
Andrew> the use of -Werror for these three simulators. With this done it is
Andrew> now possible to build the complete simulator tree using clang.
This seems fine to me, but I think it would be good if the comments
mentioned clang.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror
2022-10-19 15:24 ` [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Andrew Burgess
2022-10-20 3:53 ` Tsukasa OI
2022-10-20 18:36 ` Tom Tromey
@ 2022-10-23 14:17 ` Mike Frysinger
2 siblings, 0 replies; 34+ messages in thread
From: Mike Frysinger @ 2022-10-23 14:17 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> When building the cris, m32c, and sh simulators with Clang I am seeing
> build warnings from a few objects. These three simulators currently
> build with -Werror, and so these warnings cause the build to fail.
>
> When built with gcc I don't see any warnings from these targets, so
> the -Werror is fine.
>
> As the warnings are not new, in this commit, I propose that we disable
> the use of -Werror for these three simulators. With this done it is
> now possible to build the complete simulator tree using clang.
sory, but i don't understand the logic here. the code builds cleanly with gcc
which is why we have -Werror enabled. but when building with clang, you see
errors, so you want to disable -Werror for gcc and allow new issues to slip in
to the tree ?
this change/approach looks wrong to me. if the tree is clean with gcc, leave
it to people interested in clang to figure it out w/out making the situation
worse for GNU users. this is the GNU sim, not the LLVM sim.
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/10] Building the sim/ tree with clang
2022-10-19 15:24 [PATCH 00/10] Building the sim/ tree with clang Andrew Burgess
` (9 preceding siblings ...)
2022-10-19 15:24 ` [PATCH 10/10] sim/cris/m32c/sh: disable use of -Werror Andrew Burgess
@ 2022-10-20 18:36 ` Tom Tromey
10 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2022-10-20 18:36 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> The goal of this series is to allow the simulator tree to build using
Andrew> clang.
Andrew> The first 9 patches resolve some minor build warnings.
Andrew> The last patch is where I give up (for now), and disable -Werror for 3
Andrew> simulators which I think are going to be trickier to fix.
I can't really comment on patch #9, but otherwise these all looked fine
to me. Thanks for doing this.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread