From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130145 invoked by alias); 17 Mar 2015 08:06:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 130135 invoked by uid 89); 17 Mar 2015 08:06:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: smtp.gentoo.org Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 17 Mar 2015 08:06:56 +0000 Received: from vapier (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with SMTP id 6A81F3409D2; Tue, 17 Mar 2015 08:06:54 +0000 (UTC) Date: Tue, 17 Mar 2015 08:06:00 -0000 From: Mike Frysinger To: Jiri Gaisler Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 03/14] sim/erc32: Switched emulated memory to host endian order. Message-ID: <20150317080653.GD8109@vapier> Mail-Followup-To: Jiri Gaisler , gdb-patches@sourceware.org References: <1425244244-27709-1-git-send-email-jiri@gaisler.se> <1425244244-27709-4-git-send-email-jiri@gaisler.se> <20150302011342.GH19363@vapier> <55020453.9080502@gaisler.se> <20150312235507.GG877@vapier> <55029ED9.5070009@gaisler.se> <20150314074532.GX877@vapier> <5503FDF9.8000109@gaisler.se> <20150314102356.GZ877@vapier> <55049DBE.5080204@gaisler.se> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WChQLJJJfbwij+9x" Content-Disposition: inline In-Reply-To: <55049DBE.5080204@gaisler.se> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00475.txt.bz2 --WChQLJJJfbwij+9x Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-length: 4599 On 14 Mar 2015 21:44, Jiri Gaisler wrote: > On 14/03/15 11:23, Mike Frysinger wrote: > > On 14 Mar 2015 10:23, Jiri Gaisler wrote: > >> On 14/03/15 08:45, Mike Frysinger wrote: > >>> On 13 Mar 2015 09:24, Jiri Gaisler wrote: > >>>> On 13/03/15 00:55, Mike Frysinger wrote: > >>>>> On 12 Mar 2015 22:25, Jiri Gaisler wrote: > >>>>>> On 02/03/15 02:13, Mike Frysinger wrote: > >>>>>>>> +#ifdef HOST_LITTLE_ENDIAN > >>>>>>>>> + for (i =3D 0; i < (count / 4); i++) wbuffer[i] =3D ntohl= (wbuffer[i]); // endian swap > >>>>>>>>> +#endif > >>>>>>> > >>>>>>> sim-endian.h already provides a lot of helper funcs that i'm pret= ty sure you=20 > >>>>>>> can use here. > >>>>>> > >>>>>> I don't understand why ntohl() is a problem. It is a common Posix = function > >>>>>> that converts big endian to host endian, exactly what is needed. U= sing > >>>>>> sim-endian.h pulls in a lot of the sim-*.c files due to dependenci= es and > >>>>>> makes the simulator larger than necessary .... > >>>>> > >>>>> "network" has no meaning here. using it as a proxy for moving betw= een big=20 > >>>>> endian and native endian when there are clear functions that the si= m has=20 > >>>>> standardized on isn't correct. your code also (1) requires duplica= ting branches=20 > >>>>> and (2) inline preprocessor checks. it also does not properly hand= le bi-endian=20 > >>>>> builds. sim-endian does all of these for you. the whole point of = common/ is=20 > >>>>> to delete code from each sim rather than open coding it everywhere. > >>>>> > >>>>> wrt size, i don't think that's a compelling argument. we're talkin= g units of=20 > >>>>> KiB here, and i can't even count that low :P. > >>>>> > >>>>> if you're having trouble converting the build over (compiling/linki= ng errors),=20 > >>>>> then we can discuss that. but it'd be a matter of "do we do it now= or later"=20 > >>>>> rather than "do we do ever convert". > >>>> > >>>> Right. I tried to use the T2H_4 macro, but can't get it to compile. > >>>> I included and added sim-endian.o and sim-io.o to the > >>>> Makefile, but it complains about unresolved function etc. Do I really > >>>> need to create a sim-main.c and sim-main.h just to use T2H? > >>> > >>> i've pushed a patch to bury the sim-io.h include in sim-assert.h (sin= ce that's=20 > >>> the header that actually uses the sim_io_xxx funcs). if you define y= our own=20 > >>> ASSERT/SIM_ASSERT macros, it should be avoided for now. just make su= re you add=20 > >>> a note that they should get converted to sim-assert.h at some point. > >> > >> I'm not sure this helps. sim-endian.c includes sim-assert.h, so I get = the > >> same problem even after your patch: > >> > >> gcc -DHAVE_CONFIG_H -DPROFILE=3D1 -DWITH_PROFILE=3D-1 -DWI= TH_HOST_BYTE_ORDER=3DLITTLE_ENDIAN -DDEFAULT_INLINE=3D0 -DFAST_UA= RT -I../../../../../ibm/src/gdb/binutils-gdb/sim/erc32/../.. -I. -I../../= ../../../ibm/src/gdb/binutils-gdb/sim/erc32 -I../common > >> -I../../../../../ibm/src/gdb/binutils-gdb/sim/erc32/../common -I../../= include -I../../../../../ibm/src/gdb/binutils-gdb/sim/erc32/../../include -= I../../bfd -I../../../../../ibm/src/gdb/binutils-gdb/sim/erc32/../../bfd -I= ../../opcodes > >> -I../../../../../ibm/src/gdb/binutils-gdb/sim/erc32/../../opcodes -g = -O2 -static-libstdc++ -static-libgcc -o run \ > >> run.o libsim.a ../../bfd/libbfd.a ../../opcodes/libopcodes.a ../..= /libiberty/libiberty.a -ltermcap -ldl -lz -lnsl ../../readline/libreadline= .a -ltermcap -lm > >> libsim.a(sim-endian.o): In function `offset_1': > >> /home/jiri/src/gdb/v4/sim/erc32/../../../../../ibm/src/gdb/binutils-gd= b/sim/erc32/../common/sim-n-endian.h:145: undefined reference to `sim_io_er= ror' > >=20 > > did you define ASSERT/SIM_ASSERT before including sim-endian.h ? >=20 > No, I included sim-basic.h in my code. Including sim-endian.h only will n= ot > work due to dependencies on other include files. I don't see how this will > change how sim-endian.o is built though, as it is compiled separately. >=20 > I did manage to compile the code by including sim-endian.c directly into = my > own code (func.c) rather then building it separately: >=20 > #include > #undef ASSERT > #define ASSERT(x) if (0) {} > #include >=20 > Is this acceptable ...? lets go with your first patch with some /* TODO */ added in these areas. w= e=20 need to some more clean up in this sim and lay some basic ground work befor= e we=20 can have it start using common/. i didn't realize just how disconnected er= c32=20 was from everything else. -mike --WChQLJJJfbwij+9x Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-length: 819 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJVB+CdAAoJEEFjO5/oN/WB0YcP/34wqQ5JPPNquH7Emkgajl+s lzK+Pulo/44JeutJNmQkfLekfjg0qVmyMGhMSghX1gnUo5nhizj5GMp2bPPAtEmj 3Nq8FUSquylACiENWw9RX6OlrAPMbyVry25D7YTNoAhNtAAX8wMLaW7SWjlTLT67 Hov1FI6z+cxiuDIcjm7lidlsvtnlmHUaTkAzA8wOmpVle9YYeOgfBKryrGufRy2P 8jDWxoZ1cooRkCuV4u65iJ281VMrYkvdj8o/b4YLwAeUy7H+lP/5DLdlKak3M5Rl bf6sc7M1SrE9q9ueTP9t5puetHe/2E4Hqi0U4RTVdj0lyCeb7aPTj2+n0ocYoNPE 5BiZtPbwvfr/5bHnhOOoS+19JeLNCFOtfbTj47gLP19KV7CNZmo82kW2IbzZKElX 5jLnByV+kRpEpod2LOPLcjKSNLncOLwtICz1q5HHFZ+jMnY3Hs02bW60dmBgb1aQ ta/9CEh+ezTcPWh+6odiPTa0Dk4B4yrp+wXsgb/JZz6EWtJgg0lYOWXWNedoP9s/ wwdUKiUlmCwaULw+7Q2p53OqJI8XDeg6w0G6cyVVd9czg3t7R89MxbquOGNARfn1 bXmh+EH+a3ubzb8NJvNySoHg3oaZ1m4mPd4ilnUjP4nT9JQEHRLkoC5hQDUEWC8x QqinuamycBB8WwPQ/62/ =Mk0T -----END PGP SIGNATURE----- --WChQLJJJfbwij+9x--