Re: [PATCH v3 03/14] sim/erc32: Switched emulated memory to host endian order.

[Jiri Gaisler <jiri@gaisler.se>]

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



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 = 0; i < (count / 4); i++) wbuffer[i] = ntohl(wbuffer[i]); // endian swap
>>>>>>>>> +#endif
>>>>>>>
>>>>>>> sim-endian.h already provides a lot of helper funcs that i'm pretty sure you 
>>>>>>> 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. Using
>>>>>> sim-endian.h pulls in a lot of the sim-*.c files due to dependencies and
>>>>>> makes the simulator larger than necessary ....
>>>>>
>>>>> "network" has no meaning here.  using it as a proxy for moving between big 
>>>>> endian and native endian when there are clear functions that the sim has 
>>>>> standardized on isn't correct.  your code also (1) requires duplicating branches 
>>>>> and (2) inline preprocessor checks.  it also does not properly handle bi-endian 
>>>>> builds.  sim-endian does all of these for you.  the whole point of common/ is 
>>>>> 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 talking units of 
>>>>> KiB here, and i can't even count that low :P.
>>>>>
>>>>> if you're having trouble converting the build over (compiling/linking errors), 
>>>>> then we can discuss that.  but it'd be a matter of "do we do it now or later" 
>>>>> 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 <sim-basics.h> 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 (since that's 
>>> the header that actually uses the sim_io_xxx funcs).  if you define your own 
>>> ASSERT/SIM_ASSERT macros, it should be avoided for now.  just make sure you add 
>>> 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=1 -DWITH_PROFILE=-1         -DWITH_HOST_BYTE_ORDER=LITTLE_ENDIAN -DDEFAULT_INLINE=0           -DFAST_UART -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-gdb/sim/erc32/../common/sim-n-endian.h:145: undefined reference to `sim_io_error'
> 
> did you define ASSERT/SIM_ASSERT before including sim-endian.h ?

No, I included sim-basic.h in my code. Including sim-endian.h only will not
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.

I did manage to compile the code by including sim-endian.c directly into my
own code (func.c) rather then building it separately:

#include <sim-assert.h>
#undef  ASSERT
#define ASSERT(x) if (0) {}
#include <sim-endian.c>

Is this acceptable ...?

Jiri.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]