Re: [PATCH] desc-cpu.scm: support passing the instruction endianness to cgen_cpu_open

* Jose E. Marchesi via Cgen <[email protected]> [2020-05-19 14:45:41 +0200]:

> 
> Hi people!
> 
> This patch adds support for specifying the "instruction endianness" (as
> oppossed to data endianness) when calling cgen_cpu_open.
> 
> This is part of a bigger work to properly support arches like BPF, where
> the endianness of the instruction is different to the endianness of the
> contents of the instruction's fields.
> 
> The accompanying patch for opcodes will be sent to
> [email protected] today.  This CGEN patch will have to be applied
> first though.
> 
> OK for master?

I'm not a maintainer so can't approve the patch, but for what it's
worth, this looks good to me.

Thanks,
Andrew



>     
> 2020-05-19  Jose E. Marchesi  <[email protected]>
> 
> 	* desc-cpu.scm (/gen-cpu-open): Support passing the instruction
> 	endianness to cgen_cpu_open.
> 
> 
> diff --git a/desc-cpu.scm b/desc-cpu.scm
> index b24c9f2..e00d8cd 100644
> --- a/desc-cpu.scm
> +++ b/desc-cpu.scm
> @@ -788,6 +788,7 @@ static void
>     CGEN_CPU_OPEN_MACHS:   bitmap of values in enum mach_attr
>     CGEN_CPU_OPEN_BFDMACH: specify 1 mach using bfd name
>     CGEN_CPU_OPEN_ENDIAN:  specify endian choice
> +   CGEN_CPU_OPEN_INSN_ENDIAN: specify instruction endian choice
>     CGEN_CPU_OPEN_END:     terminates arguments
>  
>     ??? Simultaneous multiple isas might not make sense, but it's not (yet)
> @@ -801,6 +802,7 @@ CGEN_CPU_DESC
>    CGEN_BITSET *isas = 0;  /* 0 = \"unspecified\" */
>    unsigned int machs = 0; /* 0 = \"unspecified\" */
>    enum cgen_endian endian = CGEN_ENDIAN_UNKNOWN;
> +  enum cgen_endian insn_endian = CGEN_ENDIAN_UNKNOWN;
>    va_list ap;
>  
>    if (! init_p)
> @@ -835,6 +837,9 @@ CGEN_CPU_DESC
>  	case CGEN_CPU_OPEN_ENDIAN :
>  	  endian = va_arg (ap, enum cgen_endian);
>  	  break;
> +	case CGEN_CPU_OPEN_INSN_ENDIAN :
> +	  insn_endian = va_arg (ap, enum cgen_endian);
> +	  break;
>  	default :
>  	  opcodes_error_handler
>  	    (/* xgettext:c-format */
> @@ -864,11 +869,8 @@ CGEN_CPU_DESC
>    cd->isas = cgen_bitset_copy (isas);
>    cd->machs = machs;
>    cd->endian = endian;
> -  /* FIXME: for the sparc case we can determine insn-endianness statically.
> -     The worry here is where both data and insn endian can be independently
> -     chosen, in which case this function will need another argument.
> -     Actually, will want to allow for more arguments in the future anyway.  */
> -  cd->insn_endian = endian;
> +  if (insn_endian == CGEN_ENDIAN_UNKNOWN)
> +    cd->insn_endian = endian;
>  
>    /* Table (re)builder.  */
>    cd->rebuild_tables = @arch@_cgen_rebuild_tables;