[1/4] opcodes: use CGEN_INSN_LGUINT for base instructions
Checks
Commit Message
This patch changes the opcodes CGEN support code in order to allow
base instructions with opcodes past the least significative 32 bits.
Note that the masks have been adapted in a previous patch.
This patch has been regtested for all the current targets in binutils
that are based on CGEN, namely:
- bpf-unknown-none
- lm32-elf
- fr30-elf
- ip2k-elf
- iq2000-elf
- m32c-elf
- m32r-elf
- mep-elf
- mt-elf
- or1k-elf
- stormy16-elf"
Also with --enable-cgen-maint and --enable-targets=all.
No regressions observed.
include/ChangeLog:
2023-05-10 Jose E. Marchesi <jose.marchesi@oracle.com>
* opcode/cgen.h (CGEN_IVALUE): Make room for 64-bit base values.
opcodes/ChangeLog:
2023-05-10 Jose E. Marchesi <jose.marchesi@oracle.com>
* cgen-dis.in (print_insn): Use CGEN_INSN_LGUINT for instruction
base values.
* cgen-dis.c (cgen_dis_lookup_insn): Likewise.
* cgen-opc.c (cgen_macro_insn_count): Likewise.
* epiphany-dis.c: Regenerate.
* fr30-dis.c: Likewise.
* frv-dis.c: Likewise.
* ip2k-dis.c: Likewise.
* iq2000-dis.c: Likewise.
* lm32-dis.c: Likewise.
* m32c-dis.c: Likewise.
* m32r-dis.c: Likewise.
* mep-dis.c: Likewise.
* mt-dis.c: Likewise.
* or1k-dis.c: Likewise.
* xstormy16-dis.c: Likewise.
---
include/ChangeLog | 4 ++++
include/opcode/cgen.h | 10 +++++-----
opcodes/ChangeLog | 19 +++++++++++++++++++
opcodes/bpf-dis.c | 2 +-
opcodes/cgen-dis.c | 2 +-
opcodes/cgen-dis.in | 2 +-
opcodes/cgen-opc.c | 8 ++++----
opcodes/epiphany-dis.c | 2 +-
opcodes/fr30-dis.c | 2 +-
opcodes/frv-dis.c | 2 +-
opcodes/ip2k-dis.c | 2 +-
opcodes/iq2000-dis.c | 2 +-
opcodes/lm32-dis.c | 2 +-
opcodes/m32c-dis.c | 2 +-
opcodes/m32r-dis.c | 2 +-
opcodes/mep-dis.c | 2 +-
opcodes/mt-dis.c | 2 +-
opcodes/or1k-dis.c | 2 +-
opcodes/xstormy16-dis.c | 2 +-
19 files changed, 47 insertions(+), 24 deletions(-)
Comments
On 11.05.2023 16:13, Jose E. Marchesi via Binutils wrote:
> --- a/include/opcode/cgen.h
> +++ b/include/opcode/cgen.h
I will admit that I haven't looked much at cgen code before. Still two
points: For one I'm somewhat surprised you get away without also
changing CGEN_INSN_BYTES{,_PTR}. And then ...
> @@ -928,7 +928,7 @@ typedef struct
> typedef struct
> {
> /* The opcode portion of the base insn. */
> - CGEN_INSN_INT base_value;
> + CGEN_INSN_LGUINT base_value;
>
> #ifdef CGEN_MAX_EXTRA_OPCODE_OPERANDS
> /* Extra opcode values beyond base_value. */
> @@ -1186,7 +1186,7 @@ extern CGEN_INSN_LIST * cgen_asm_lookup_insn
> instruction (the actually hashing done is up to the target). */
>
> extern CGEN_INSN_LIST * cgen_dis_lookup_insn
> - (CGEN_CPU_DESC, const char *, CGEN_INSN_INT);
> + (CGEN_CPU_DESC, const char *, CGEN_INSN_LGUINT);
> /* FIXME: delete these two */
> #define CGEN_DIS_LOOKUP_INSN(cd, buf, value) cgen_dis_lookup_insn ((cd), (buf), (value))
> #define CGEN_DIS_NEXT_INSN(insn) ((insn)->next)
> @@ -1449,7 +1449,7 @@ extern int CGEN_SYM (get_mach) (const char *);
> /* Operand index computation. */
> extern const CGEN_INSN * cgen_lookup_insn
> (CGEN_CPU_DESC, const CGEN_INSN * insn_,
> - CGEN_INSN_INT int_value_, unsigned char *bytes_value_,
> + CGEN_INSN_LGUINT int_value_, unsigned char *bytes_value_,
> int length_, CGEN_FIELDS *fields_, int alias_p_);
> extern void cgen_get_insn_operands
> (CGEN_CPU_DESC, const CGEN_INSN * insn_,
> @@ -1461,10 +1461,10 @@ extern const CGEN_INSN * cgen_lookup_get_insn_operands
>
> /* Cover fns to bfd_get/set. */
>
> -extern CGEN_INSN_INT cgen_get_insn_value
> +extern CGEN_INSN_LGUINT cgen_get_insn_value
> (CGEN_CPU_DESC, unsigned char *, int, int);
> extern void cgen_put_insn_value
> - (CGEN_CPU_DESC, unsigned char *, int, CGEN_INSN_INT, int);
> + (CGEN_CPU_DESC, unsigned char *, int, CGEN_INSN_LGUINT, int);
>
> extern CGEN_INSN_INT cgen_get_base_insn_value
> (CGEN_CPU_DESC, unsigned char *, int);
... all the type adjustments are merely to scalars, not to pointers.
Therefore it's not really clear to me why you need to change the
types of local variables in at least one function for all the targets
using cgen. I find it somewhat undesirable to force them all to now
act on 64-bit quantities. I would even wonder if this need couldn't
be abstracted enough (along the lines of CGEN_INSN_BYTES) such that
only bpf would need to start dealing with 64-bit values.
Jan
Hi Jose,
> This patch changes the opcodes CGEN support code in order to allow
> base instructions with opcodes past the least significative 32 bits.
>
> Note that the masks have been adapted in a previous patch.
>
> This patch has been regtested for all the current targets in binutils
> that are based on CGEN, namely:
>
> - bpf-unknown-none
> - lm32-elf
> - fr30-elf
> - ip2k-elf
> - iq2000-elf
> - m32c-elf
> - m32r-elf
> - mep-elf
> - mt-elf
> - or1k-elf
> - stormy16-elf"
>
> Also with --enable-cgen-maint and --enable-targets=all.
> No regressions observed.
Patch approved - please apply.
Cheers
Nick
Hi Nick.
> Hi Jose,
>
>> This patch changes the opcodes CGEN support code in order to allow
>> base instructions with opcodes past the least significative 32 bits.
>> Note that the masks have been adapted in a previous patch.
>> This patch has been regtested for all the current targets in
>> binutils
>> that are based on CGEN, namely:
>> - bpf-unknown-none
>> - lm32-elf
>> - fr30-elf
>> - ip2k-elf
>> - iq2000-elf
>> - m32c-elf
>> - m32r-elf
>> - mep-elf
>> - mt-elf
>> - or1k-elf
>> - stormy16-elf"
>> Also with --enable-cgen-maint and --enable-targets=all.
>> No regressions observed.
>
> Patch approved - please apply.
Thanks.
Before pushing I am looking at the CGEN_INSN_BYTES point raised by Jan,
which made me realize that, even when I am pretty sure the path
introduces no regressions, it may be incomplete because it won't cover
targets that would have opcodes past the lowest 32-bits of the base
instruction and that also define CGEN_INT_INSN_P (even if we currently
don't have such targets.)
On 5/17/23 07:03, Jose E. Marchesi via Binutils wrote:
>
> Hi Nick.
>
>> Hi Jose,
>>
>>> This patch changes the opcodes CGEN support code in order to allow
>>> base instructions with opcodes past the least significative 32 bits.
>>> Note that the masks have been adapted in a previous patch.
>>> This patch has been regtested for all the current targets in
>>> binutils
>>> that are based on CGEN, namely:
>>> - bpf-unknown-none
>>> - lm32-elf
>>> - fr30-elf
>>> - ip2k-elf
>>> - iq2000-elf
>>> - m32c-elf
>>> - m32r-elf
>>> - mep-elf
>>> - mt-elf
>>> - or1k-elf
>>> - stormy16-elf"
>>> Also with --enable-cgen-maint and --enable-targets=all.
>>> No regressions observed.
>>
>> Patch approved - please apply.
>
> Thanks.
>
> Before pushing I am looking at the CGEN_INSN_BYTES point raised by Jan,
> which made me realize that, even when I am pretty sure the path
> introduces no regressions, it may be incomplete because it won't cover
> targets that would have opcodes past the lowest 32-bits of the base
> instruction and that also define CGEN_INT_INSN_P (even if we currently
> don't have such targets.)
With the recent changes to the BPF port to not use CGEN, this patch
is no longer needed, right Jose?
> On 5/17/23 07:03, Jose E. Marchesi via Binutils wrote:
>>
>> Hi Nick.
>>
>>> Hi Jose,
>>>
>>>> This patch changes the opcodes CGEN support code in order to allow
>>>> base instructions with opcodes past the least significative 32 bits.
>>>> Note that the masks have been adapted in a previous patch.
>>>> This patch has been regtested for all the current targets in
>>>> binutils
>>>> that are based on CGEN, namely:
>>>> - bpf-unknown-none
>>>> - lm32-elf
>>>> - fr30-elf
>>>> - ip2k-elf
>>>> - iq2000-elf
>>>> - m32c-elf
>>>> - m32r-elf
>>>> - mep-elf
>>>> - mt-elf
>>>> - or1k-elf
>>>> - stormy16-elf"
>>>> Also with --enable-cgen-maint and --enable-targets=all.
>>>> No regressions observed.
>>>
>>> Patch approved - please apply.
>>
>> Thanks.
>>
>> Before pushing I am looking at the CGEN_INSN_BYTES point raised by Jan,
>> which made me realize that, even when I am pretty sure the path
>> introduces no regressions, it may be incomplete because it won't cover
>> targets that would have opcodes past the lowest 32-bits of the base
>> instruction and that also define CGEN_INT_INSN_P (even if we currently
>> don't have such targets.)
>
> With the recent changes to the BPF port to not use CGEN, this patch
> is no longer needed, right Jose?
Correct. To no have to deal with these kind of tangents was a big
motivation for the desCGENization of the port.
@@ -1,3 +1,7 @@
+2023-05-10 Jose E. Marchesi <jose.marchesi@oracle.com>
+
+ * opcode/cgen.h (CGEN_IVALUE): Make room for 64-bit base values.
+
2023-03-23 Frederic Cambus <fred@statdns.com>
* elf/common.h (PT_OPENBSD_MUTABLE): Define.
@@ -928,7 +928,7 @@ typedef struct
typedef struct
{
/* The opcode portion of the base insn. */
- CGEN_INSN_INT base_value;
+ CGEN_INSN_LGUINT base_value;
#ifdef CGEN_MAX_EXTRA_OPCODE_OPERANDS
/* Extra opcode values beyond base_value. */
@@ -1186,7 +1186,7 @@ extern CGEN_INSN_LIST * cgen_asm_lookup_insn
instruction (the actually hashing done is up to the target). */
extern CGEN_INSN_LIST * cgen_dis_lookup_insn
- (CGEN_CPU_DESC, const char *, CGEN_INSN_INT);
+ (CGEN_CPU_DESC, const char *, CGEN_INSN_LGUINT);
/* FIXME: delete these two */
#define CGEN_DIS_LOOKUP_INSN(cd, buf, value) cgen_dis_lookup_insn ((cd), (buf), (value))
#define CGEN_DIS_NEXT_INSN(insn) ((insn)->next)
@@ -1449,7 +1449,7 @@ extern int CGEN_SYM (get_mach) (const char *);
/* Operand index computation. */
extern const CGEN_INSN * cgen_lookup_insn
(CGEN_CPU_DESC, const CGEN_INSN * insn_,
- CGEN_INSN_INT int_value_, unsigned char *bytes_value_,
+ CGEN_INSN_LGUINT int_value_, unsigned char *bytes_value_,
int length_, CGEN_FIELDS *fields_, int alias_p_);
extern void cgen_get_insn_operands
(CGEN_CPU_DESC, const CGEN_INSN * insn_,
@@ -1461,10 +1461,10 @@ extern const CGEN_INSN * cgen_lookup_get_insn_operands
/* Cover fns to bfd_get/set. */
-extern CGEN_INSN_INT cgen_get_insn_value
+extern CGEN_INSN_LGUINT cgen_get_insn_value
(CGEN_CPU_DESC, unsigned char *, int, int);
extern void cgen_put_insn_value
- (CGEN_CPU_DESC, unsigned char *, int, CGEN_INSN_INT, int);
+ (CGEN_CPU_DESC, unsigned char *, int, CGEN_INSN_LGUINT, int);
extern CGEN_INSN_INT cgen_get_base_insn_value
(CGEN_CPU_DESC, unsigned char *, int);
@@ -1,3 +1,22 @@
+2023-05-10 Jose E. Marchesi <jose.marchesi@oracle.com>
+
+ * cgen-dis.in (print_insn): Use CGEN_INSN_LGUINT for instruction
+ base values.
+ * cgen-dis.c (cgen_dis_lookup_insn): Likewise.
+ * cgen-opc.c (cgen_macro_insn_count): Likewise.
+ * epiphany-dis.c: Regenerate.
+ * fr30-dis.c: Likewise.
+ * frv-dis.c: Likewise.
+ * ip2k-dis.c: Likewise.
+ * iq2000-dis.c: Likewise.
+ * lm32-dis.c: Likewise.
+ * m32c-dis.c: Likewise.
+ * m32r-dis.c: Likewise.
+ * mep-dis.c: Likewise.
+ * mt-dis.c: Likewise.
+ * or1k-dis.c: Likewise.
+ * xstormy16-dis.c: Likewise.
+
2023-04-21 Tom Tromey <tromey@adacore.com>
* i386-dis.c (OP_J): Check result of get16.
@@ -367,7 +367,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -232,7 +232,7 @@ build_dis_hash_table (CGEN_CPU_DESC cd)
/* Return the first entry in the hash list for INSN. */
CGEN_INSN_LIST *
-cgen_dis_lookup_insn (CGEN_CPU_DESC cd, const char * buf, CGEN_INSN_INT value)
+cgen_dis_lookup_insn (CGEN_CPU_DESC cd, const char * buf, CGEN_INSN_LGUINT value)
{
unsigned int hash;
@@ -202,7 +202,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -355,13 +355,13 @@ cgen_macro_insn_count (CGEN_CPU_DESC cd)
/* Cover function to read and properly byteswap an insn value. */
-CGEN_INSN_INT
+CGEN_INSN_LGUINT
cgen_get_insn_value (CGEN_CPU_DESC cd, unsigned char *buf, int length,
int endian)
{
int big_p = (endian == CGEN_ENDIAN_BIG);
int insn_chunk_bitsize = cd->insn_chunk_bitsize;
- CGEN_INSN_INT value = 0;
+ CGEN_INSN_LGUINT value = 0;
if (insn_chunk_bitsize != 0 && insn_chunk_bitsize < length)
{
@@ -397,7 +397,7 @@ void
cgen_put_insn_value (CGEN_CPU_DESC cd,
unsigned char *buf,
int length,
- CGEN_INSN_INT value,
+ CGEN_INSN_LGUINT value,
int endian)
{
int big_p = (endian == CGEN_ENDIAN_BIG);
@@ -446,7 +446,7 @@ cgen_put_insn_value (CGEN_CPU_DESC cd,
const CGEN_INSN *
cgen_lookup_insn (CGEN_CPU_DESC cd,
const CGEN_INSN *insn,
- CGEN_INSN_INT insn_int_value,
+ CGEN_INSN_LGUINT insn_int_value,
/* ??? CGEN_INSN_BYTES would be a nice type name to use here. */
unsigned char *insn_bytes_value,
int length,
@@ -443,7 +443,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -464,7 +464,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -561,7 +561,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -453,7 +453,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -354,7 +354,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -312,7 +312,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -1056,7 +1056,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -444,7 +444,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -1366,7 +1366,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -452,7 +452,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -339,7 +339,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;
@@ -333,7 +333,7 @@ print_insn (CGEN_CPU_DESC cd,
bfd_byte *buf,
unsigned int buflen)
{
- CGEN_INSN_INT insn_value;
+ CGEN_INSN_LGUINT insn_value;
const CGEN_INSN_LIST *insn_list;
CGEN_EXTRACT_INFO ex_info;
int basesize;