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

[Mike Frysinger <vapier@gentoo.org>]

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

On 01 Mar 2015 22:10, Jiri Gaisler wrote:
> --- a/sim/erc32/erc32.c
> +++ b/sim/erc32/erc32.c
>
>      if ((addr >= mem_ramstart) && (addr < (mem_ramstart + mem_ramsz))) {
> -	fetch_bytes (asi, &ramb[addr & mem_rammask], data, sz);
> +        *data = *((uint32 *) & (ramb[addr & mem_rammask & ~3]));

pretty sure this too should be a memcpy.  also applies to the other updates to 
this func in this patch.

> --- a/sim/erc32/exec.c
> +++ b/sim/erc32/exec.c
>  
> +static int
> +extract_short (uint32 data, uint32 address)
> +{
> +    return((data >> ((2 - (address & 2)) * 8)) & 0xffff);

needs to be a space after the return

> +static int
> +extract_short_signed (uint32 data, uint32 address)
> +{
> +    uint32 tmp;
> +    tmp = ((data >> ((2 - (address & 2)) * 8)) & 0xffff);

you could merge these two statements if you wanted

> +    if (tmp & 0x8000) tmp |= 0xffff0000;

uncuddle this

> +    return(tmp);

drop the paren

> +static int
> +extract_byte (uint32 data, uint32 address)
> +{
> +    return((data >> ((3 - (address & 3)) * 8)) & 0xff);

space after the return

> +static int
> +extract_byte_signed (uint32 data, uint32 address)
> +{
> +    uint32 tmp;
> +    tmp = ((data >> ((3 - (address & 3)) * 8)) & 0xff);

merge if you want

> +    if (tmp & 0x80) tmp |= 0xffffff00;

uncuddle

> +    return(tmp);

drop paren

> --- a/sim/erc32/func.c
> +++ b/sim/erc32/func.c
>
> +	    if (isprint(p[j^end]))
> +		putchar(p[j^end]);

spaces around that ^ operator

> @@ -841,10 +850,11 @@ dis_mem(addr, len, info)
>  {
>      uint32          i;
>      unsigned char   data[4];
> +    uint32          *wdata = (uint32 *) data;

use a union ? :)

>  		while (section_size > 0) {
> -		    char            buffer[1024];
>  		    int             count;
> +		    char            buffer[1024];
> +		    uint32          *wbuffer = (uint32 *) buffer;

use a union

> +#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.

> @@ -356,7 +360,19 @@ sim_write(sd, mem, buf, length)
>      const unsigned char  *buf;
>      int             length;
>  {
> +#ifdef HOST_LITTLE_ENDIAN
> +    int *ibufp = (int *) buf;
> +    int ibuf[8192];
> +    int i, len;
> +
> +    if (length >= 4)
> +      for (i = 0; i < length; i += 4) {
> +        ibuf[i] = ntohl(ibufp[i]);
> +      }
> +    return (sis_memory_write(mem, (char *) ibuf, length));
> +#else
>      return (sis_memory_write(mem, buf, length));
> +#endif

same here

> @@ -366,7 +382,20 @@ sim_read(sd, mem, buf, length)
>       unsigned char *buf;
>       int length;
>  {
> +#ifdef HOST_LITTLE_ENDIAN
> +    int *ibuf = (int *) buf;
> +    int i, len;
> +
> +    len = sis_memory_read(mem, buf, length);
> +    if (length >= 4)
> +      for (i = 0; i < length; i += 4) {
> +        *ibuf = htonl(*ibuf);
> +        ibuf++;
> +      }
> +    return (len);
> +#else
>      return (sis_memory_read(mem, buf, length));
> +#endif

and here
-mike

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