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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - cgen list mailing list

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
[hidden email] today.  This CGEN patch will have to be applied
first though.

OK for master?
   
2020-05-19  Jose E. Marchesi  <[hidden email]>

        * 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;
Reply | Threaded
Open this post in threaded view
|

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

Andrew Burgess
* Jose E. Marchesi via Cgen <[hidden email]> [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
> [hidden email] 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  <[hidden email]>
>
> * 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;
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - cgen list mailing list
In reply to this post by Sourceware - cgen list mailing list
Hi -

> The accompanying patch for opcodes will be sent to
> [hidden email] today.  This CGEN patch will have to be applied
> first though.
>
> OK for master?

Sure, and it seems to have been anticipated.


- FChE

Reply | Threaded
Open this post in threaded view
|

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

Jose E. Marchesi
   
    > The accompanying patch for opcodes will be sent to
    > [hidden email] today.  This CGEN patch will have to be applied
    > first though.
    >
    > OK for master?
   
    Sure, and it seems to have been anticipated.
   
Thanks.  I will push once the binutils stuff is also ready... found a
complication.

(Actually, the whole handling of endianness in both CGEN and binutils is
 a mess.  Trying to get some sense of it.)
 
Reply | Threaded
Open this post in threaded view
|

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

Jose E. Marchesi

       
        > The accompanying patch for opcodes will be sent to
        > [hidden email] today.  This CGEN patch will have to be applied
        > first though.
        >
        > OK for master?
       
        Sure, and it seems to have been anticipated.
       
    Thanks.  I will push once the binutils stuff is also ready... found a
    complication.

Ok I think I got the binutils side nailed.  Will post the patches to
binutils@sourceware once regression testing finishes running, later
today.

After the binutils part is approved I will push the following (fixed)
version of the CGEN patch, to master:

commit b684299385722879c5658f99cdbe0c63a8bc7362 (HEAD -> jemarch/cgen-insn-endian, malditobastardo/jemarch/cgen-insn-endian)
Author: Jose E. Marchesi <[hidden email]>
Date:   Fri May 29 16:03:48 2020 +0200

    desc-cpu.scm: support passing the instruction endianness to cgen_cpu_open
   
    2020-05-29  Jose E. Marchesi  <[hidden email]>
   
            * desc-cpu.scm (/gen-cpu-open): Support passing the instruction
            endianness to cgen_cpu_open.

diff --git a/ChangeLog b/ChangeLog
index e554fa2..93eafe4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-29  Jose E. Marchesi  <[hidden email]>
+
+ * desc-cpu.scm (/gen-cpu-open): Support passing the instruction
+ endianness to cgen_cpu_open.
+
 2020-05-21  Alan Modra  <[hidden email]>
 
  * desc-cpu.scm (@arch@_cgen_cpu_close): Free without first
diff --git a/desc-cpu.scm b/desc-cpu.scm
index 5bf5bc7..34f5d5c 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;
+  cd->insn_endian
+    = (insn_endian == CGEN_ENDIAN_UNKNOWN ? endian : insn_endian);
 
   /* Table (re)builder.  */
   cd->rebuild_tables = @arch@_cgen_rebuild_tables;

Reply | Threaded
Open this post in threaded view
|

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

Jose E. Marchesi

           
            > The accompanying patch for opcodes will be sent to
            > [hidden email] today.  This CGEN patch will have to be applied
            > first though.
            >
            > OK for master?
           
            Sure, and it seems to have been anticipated.
           
        Thanks.  I will push once the binutils stuff is also ready... found a
        complication.
   
    Ok I think I got the binutils side nailed.  Will post the patches to
    binutils@sourceware once regression testing finishes running, later
    today.
   
    After the binutils part is approved I will push the following (fixed)
    version of the CGEN patch, to master:

Patch pushed to master.
Salud!