Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Luis Machado (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Andrew Burgess <andrew.burgess@embecosm.com>,	gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Subject: [review v4] [ARM, sim] Fix build error and warnings
Date: Tue, 03 Dec 2019 13:55:00 -0000	[thread overview]
Message-ID: <20191203135534.E064F2816F@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1574856916000.I21db699d3b61b2de8c44053e47be4387285af28f@gnutoolchain-gerrit.osci.io>

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/726
......................................................................

[ARM, sim] Fix build error and warnings

Newer GCC's have switched to -fno-common by default, and this breaks the build
for the ARM sim, like this:

binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:65: multiple definition of `DSPsc'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:134: first defined here
binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:64: multiple definition of `DSPacc'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:133: first defined here
binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:63: multiple definition of `DSPregs'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:132: first defined here

I also noticed a few warnings due to mismatching types, as follows:

../../../../repos/binutils-gdb/sim/arm/wrapper.c: In function ‘sim_create_inferior’:
../../../../repos/binutils-gdb/sim/arm/wrapper.c:335:16: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
       for (arg = argv; *arg != NULL; arg++)
                ^
../../../../repos/binutils-gdb/sim/arm/wrapper.c:342:8: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    arg = argv;
        ^
../../../../repos/binutils-gdb/sim/arm/wrapper.c:345:13: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    for (arg = argv; *arg != NULL; arg++)
             ^
The following patch fixes both of the above.

sim/arm/ChangeLog:

2019-12-03  Luis Machado  <luis.machado@linaro.org>

	* armemu.c (isize): Move this declaration ...
	* arminit.c (isize): ... here.
	* maverick.h: New file.
	* wrapper.c: Include "maverick.h".
	(<struct maverick_regs>, <union maverick_acc_regs>): Remove and update
	comment.
	(sim_create_inferior): Cast variables to proper type.
	* maverick.c: Include "maverick.h".
	(<struct maverick_regs>, <union maverick_acc_regs>): Move
	declarations to maverick.h and update comment.
	(DSPsc, DSPacc, DSPregs): Adjust comment.

Signed-off-by: Luis Machado <luis.machado@linaro.org>
Change-Id: I21db699d3b61b2de8c44053e47be4387285af28f
---
M sim/arm/armemu.c
M sim/arm/arminit.c
M sim/arm/maverick.c
A sim/arm/maverick.h
M sim/arm/wrapper.c
5 files changed, 57 insertions(+), 67 deletions(-)



diff --git a/sim/arm/armemu.c b/sim/arm/armemu.c
index 76f398b..3a72277 100644
--- a/sim/arm/armemu.c
+++ b/sim/arm/armemu.c
@@ -1140,10 +1140,6 @@
 
 /* EMULATION of ARM6.  */
 
-/* The PC pipeline value depends on whether ARM
-   or Thumb instructions are being executed.  */
-ARMword isize;
-
 ARMword
 #ifdef MODE32
 ARMul_Emulate32 (ARMul_State * state)
diff --git a/sim/arm/arminit.c b/sim/arm/arminit.c
index 851d356..3a626c8 100644
--- a/sim/arm/arminit.c
+++ b/sim/arm/arminit.c
@@ -40,6 +40,10 @@
 ARMword ARMul_ImmedTable[4096];	/* immediate DP LHS values */
 char ARMul_BitList[256];	/* number of bits in a byte table */
 
+/* The PC pipeline value depends on whether ARM
+   or Thumb instructions are being executed.  */
+ARMword isize;
+
 /***************************************************************************\
 *         Call this routine once to set up the emulator's tables.           *
 \***************************************************************************/
diff --git a/sim/arm/maverick.c b/sim/arm/maverick.c
index c112692..bae8c47 100644
--- a/sim/arm/maverick.c
+++ b/sim/arm/maverick.c
@@ -19,6 +19,7 @@
 #include "armdefs.h"
 #include "ansidecl.h"
 #include "armemu.h"
+#include "maverick.h"
 
 /*#define CIRRUS_DEBUG 1	*/
 #if CIRRUS_DEBUG
@@ -30,36 +31,10 @@
 #define POS64(i) ( (~(i)) >> 63 )
 #define NEG64(i) ( (i) >> 63 )
 
-/* Define Co-Processor instruction handlers here.  */
-
-/* Here's ARMulator's DSP definition.  A few things to note:
-   1) it has 16 64-bit registers and 4 72-bit accumulators
-   2) you can only access its registers with MCR and MRC.  */
-
-/* We can't define these in here because this file might not be linked
-   unless the target is arm9e-*.  They are defined in wrapper.c.
-   Eventually the simulator should be made to handle any coprocessor
-   at run time.  */
-struct maverick_regs
-{
-  union
-  {
-    int i;
-    float f;
-  } upper;
-
-  union
-  {
-    int i;
-    float f;
-  } lower;
-};
-
-union maverick_acc_regs
-{
-  long double ld;		/* Acc registers are 72-bits.  */
-};
-
+/* These variables are defined here and made extern in maverick.h for use
+   in wrapper.c for now.
+   Eventually the simulator should be made to handle any coprocessor at run
+   time.  */
 struct maverick_regs DSPregs[16];
 union maverick_acc_regs DSPacc[4];
 ARMword DSPsc;
diff --git a/sim/arm/maverick.h b/sim/arm/maverick.h
new file mode 100644
index 0000000..2549d21
--- /dev/null
+++ b/sim/arm/maverick.h
@@ -0,0 +1,46 @@
+/*  maverick.h -- Cirrus/DSP co-processor interface header
+    Copyright (C) 2003-2019 Free Software Foundation, Inc.
+    Contributed by Aldy Hernandez (aldyh@redhat.com).
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    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 Co-Processor instruction handlers here.  */
+
+/* Here's ARMulator's DSP definition.  A few things to note:
+   1) it has 16 64-bit registers and 4 72-bit accumulators
+   2) you can only access its registers with MCR and MRC.  */
+
+struct maverick_regs
+{
+  union
+  {
+    int i;
+    float f;
+  } upper;
+
+  union
+  {
+    int i;
+    float f;
+  } lower;
+};
+
+union maverick_acc_regs
+{
+  long double ld;		/* Acc registers are 72-bits.  */
+};
+
+extern struct maverick_regs DSPregs[16];
+extern union maverick_acc_regs DSPacc[4];
+extern ARMword DSPsc;
diff --git a/sim/arm/wrapper.c b/sim/arm/wrapper.c
index fde5d8c..78a9192 100644
--- a/sim/arm/wrapper.c
+++ b/sim/arm/wrapper.c
@@ -37,6 +37,7 @@
 #include "gdb/signals.h"
 #include "libiberty.h"
 #include "iwmmxt.h"
+#include "maverick.h"
 
 /* TODO: This should get pulled from the SIM_DESC.  */
 host_callback *sim_callback;
@@ -101,38 +102,6 @@
   fprintf (stderr, " %*s\n", size, opbuf);
 }
 
-/* Cirrus DSP registers.
-
-   We need to define these registers outside of maverick.c because
-   maverick.c might not be linked in unless --target=arm9e-* in which
-   case wrapper.c will not compile because it tries to access Cirrus
-   registers.  This should all go away once we get the Cirrus and ARM
-   Coprocessor to coexist in armcopro.c-- aldyh.  */
-
-struct maverick_regs
-{
-  union
-  {
-    int i;
-    float f;
-  } upper;
-
-  union
-  {
-    int i;
-    float f;
-  } lower;
-};
-
-union maverick_acc_regs
-{
-  long double ld;		/* Acc registers are 72-bits.  */
-};
-
-struct maverick_regs     DSPregs[16];
-union maverick_acc_regs  DSPacc[4];
-ARMword DSPsc;
-
 static void
 init (void)
 {
@@ -236,7 +205,7 @@
 {
   int argvlen = 0;
   int mach;
-  char **arg;
+  char * const *arg;
 
   init ();
 

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I21db699d3b61b2de8c44053e47be4387285af28f
Gerrit-Change-Number: 726
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset


  parent reply	other threads:[~2019-12-03 13:55 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-27 12:15 [review] " Luis Machado (Code Review)
2019-11-27 15:36 ` Simon Marchi (Code Review)
2019-11-27 16:20 ` Luis Machado (Code Review)
2019-11-27 16:54 ` Simon Marchi (Code Review)
2019-11-27 16:55 ` Simon Marchi (Code Review)
2019-11-27 18:20 ` Luis Machado (Code Review)
2019-11-28 12:12 ` Andrew Burgess (Code Review)
2019-11-28 12:38 ` Luis Machado (Code Review)
2019-11-28 13:30 ` [review v2] " Luis Machado (Code Review)
2019-11-28 13:33 ` [review v3] " Luis Machado (Code Review)
2019-12-02 22:16 ` Andrew Burgess (Code Review)
2019-12-03 13:49 ` Luis Machado (Code Review)
2019-12-03 13:55 ` Luis Machado (Code Review) [this message]
2019-12-06 10:35 ` [review v4] " Andrew Burgess (Code Review)
2019-12-06 13:09 ` Luis Machado (Code Review)
2019-12-06 13:15 ` Luis Machado (Code Review)
2019-12-06 13:21 ` Luis Machado (Code Review)
2019-12-06 14:50 ` Tom Tromey (Code Review)
2019-12-06 21:18 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-06 21:18 ` Sourceware to Gerrit sync (Code Review)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191203135534.E064F2816F@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@linaro.org \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox