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

[Mike Frysinger <vapier at gentoo dot org>]

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: signature.asc
Description: Digital signature