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

Mike Frysinger
2015-03-22T22:46:05Z

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

On 17 Mar 2015 22:02, Jiri Gaisler wrote:
> --- a/sim/erc32/erc32.c
> +++ b/sim/erc32/erc32.c
>
> static void
> -fetch_bytes (asi, mem, data, sz)
> +store_bytes (mem, waddr, data, sz, ws)

as some point we'll want to switch over to sim-core.c and then all these
accessor funcs you have would go away. but we don't need to do that now.

> +#ifdef HOST_LITTLE_ENDIAN
> + waddr ^= 3;
> +#endif
> + mem[waddr] = *data & 0x0ff;

doesn't this assume the target is big endian ? this shows up a few times in
the changes to this file.

> + memcpy(&mem[waddr], data, 2);

space before the (. this shows up a few times in the changes to this file.

> + *data = *((uint32 *) & (romb[addr & ~3]));

broken indentation, and you need to use the union/memcpy logic. this shows up a
few times in the changes to this file.

> + mexc = memory_read(asi, address, ddata, 2, &ws);

you should scan the whole change for new lines that are missing the space before
the (

> sregs.freq = freq ? freq : 15;
> termsave = fcntl(0, F_GETFL, 0);
> INIT_DISASSEMBLE_INFO(dinfo, stdout,(fprintf_ftype)fprintf);
> +#ifdef HOST_LITTLE_ENDIAN
> + dinfo.endian = BFD_ENDIAN_LITTLE;
> +#else
> dinfo.endian = BFD_ENDIAN_BIG;
> +#endif

can't you get the target endian info via the bfd library ?

> +#ifdef HOST_LITTLE_ENDIAN
> + int *ibufp = (int *) buf;

needs a union

> + int ibuf[8192];
> + int i, len;
> +
> + if (length >= 4)
> + for (i = 0; i < length; i += 4) {
> + ibuf[i] = T2H_4(ibufp[i]);
> + }
> + return (sis_memory_write(mem, (char *) ibuf, length));
> +#else
> return (sis_memory_write(mem, buf, length));
> +#endif

the point of using sim-endian is that you wouldn't have any ifdefs. code the
loop and let gcc optimize away the "nothing to do" case.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]