This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [PATCHv3 6/7] arc/opcodes: Use flag operand class to handle multiple flag matches
- From: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: "Cupertino dot Miranda at synopsys dot com" <Cupertino dot Miranda at synopsys dot com>, "noamca at mellanox dot com" <noamca at mellanox dot com>, Nick Clifton <nickc at redhat dot com>, Andreas Schwab <schwab at suse dot de>
- Date: Wed, 16 Mar 2016 11:51:20 +0000
- Subject: RE: [PATCHv3 6/7] arc/opcodes: Use flag operand class to handle multiple flag matches
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1456947552 dot git dot andrew dot burgess at embecosm dot com> <cover dot 1458082098 dot git dot andrew dot burgess at embecosm dot com> <e2edb012a06c07003a46a8bd5805a5cdd9a9c8e1 dot 1458082098 dot git dot andrew dot burgess at embecosm dot com>
It looks sane, thanks for clearing out one of the todos.
//Claudiu
> -----Original Message-----
> From: Andrew Burgess [mailto:andrew.burgess@embecosm.com]
> Sent: Wednesday, March 16, 2016 12:02 AM
> To: binutils@sourceware.org
> Cc: Claudiu.Zissulescu@synopsys.com; Cupertino.Miranda@synopsys.com;
> noamca@mellanox.com; Nick Clifton; Andreas Schwab; Andrew Burgess
> Subject: [PATCHv3 6/7] arc/opcodes: Use flag operand class to handle
> multiple flag matches
>
> When parsing the operand instruction flags we don't currently detect the
> case where multiple flags are provided from the same class set, these
> will be accepted and the bit values merged together, resulting in the
> wrong instruction being assembled. For example:
>
> adc.n.eq r0,r0,r2
>
> Will assemble without error, yet, upon disassembly, the instruction will
> actually be:
>
> adc.c r0,r0,r2
>
> In a later commit the concept of required flags will be introduced.
> Required flags are just like normal instruction flags, except that they
> must be present for the instruction to match. Adding this will allow
> for simpler instructions in the instruction table, and allow for more
> sharing of operand extraction and insertion functions.
>
> To solve both of the above issues (multiple flags being invalid, and
> required flags), this commit reworks the flag class mechanism.
> Currently the flag class is never used. Each instruction can reference
> multiple flag classes, each flag class has a class type and a set of
> flags. However, at present, the class type is never used. The current
> values identify the type of instruction that the flag will be used in,
> but this is not required information.
>
> Instead, this commit discards the old flag classes, and introduces 3 new
> classes. The first F_CLASS_NONE, is just a NULL marker value, and is
> only used in the NULL marker flag class. The other two flag classes are
> F_FLAG_OPTIONAL, and F_FLAG_REQUIRED.
>
> The class F_FLAG_OPTIONAL has the property that at most one of the flags
> in the flag set for that class must be present in the instruction. The
> "at most" one means that no flags being present is fine.
>
> The class F_FLAG_REQUIRED is not currently used, but will be soon. With
> this class, exactly one of the flags from this class must be present in
> the instruction. If the flag class contains a single flag, then of
> course that flag must be present. However, if the flag class contained
> two or more, then one, and only one of them must be present.
>
> gas/ChangeLog:
>
> * config/tc-arc.c (find_opcode_match): Move lnflg, and i
> declarations to start of block. Reset code on all flags before
> attempting to match them. Handle multiple hits on the same flag.
> Handle flag class.
> * testsuite/gas/arc/asm-errors.d: New file.
> * testsuite/gas/arc/asm-errors.err: New file.
> * testsuite/gas/arc/asm-errors.s: New file.
>
> include/ChangeLog:
>
> * opcode/arc.h (flag_class_t): Remove all old flag classes, add 3
> new classes instead.
>
> opcodes/ChangeLog:
>
> * arc-opc.c (arc_flag_classes): Convert all flag classes to use
> the new class enum values.
> ---
> gas/ChangeLog | 10 +++++++
> gas/config/tc-arc.c | 22 +++++++++++-----
> gas/testsuite/gas/arc/asm-errors.d | 2 ++
> gas/testsuite/gas/arc/asm-errors.err | 4 +++
> gas/testsuite/gas/arc/asm-errors.s | 4 +++
> include/ChangeLog | 5 ++++
> include/opcode/arc.h | 18 ++++++-------
> opcodes/ChangeLog | 5 ++++
> opcodes/arc-opc.c | 51 ++++++++++++++++++------------------
> 9 files changed, 81 insertions(+), 40 deletions(-)
> create mode 100644 gas/testsuite/gas/arc/asm-errors.d
> create mode 100644 gas/testsuite/gas/arc/asm-errors.err
> create mode 100644 gas/testsuite/gas/arc/asm-errors.s
>
> diff --git a/gas/ChangeLog b/gas/ChangeLog
> index 8df40e9..71f7ec0 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,5 +1,15 @@
> 2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
>
> + * config/tc-arc.c (find_opcode_match): Move lnflg, and i
> + declarations to start of block. Reset code on all flags before
> + attempting to match them. Handle multiple hits on the same flag.
> + Handle flag class.
> + * testsuite/gas/arc/asm-errors.d: New file.
> + * testsuite/gas/arc/asm-errors.err: New file.
> + * testsuite/gas/arc/asm-errors.s: New file.
> +
> +2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
> +
> * config/tc-arc.c (cpu_types): Add nps400 entry.
> (check_zol): Handle nps400.
>
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index 2bf7f13..736143e 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -1370,7 +1370,7 @@ find_opcode_match (const struct arc_opcode
> *first_opcode,
> {
> const unsigned char *opidx;
> const unsigned char *flgidx;
> - int tokidx = 0;
> + int tokidx = 0, lnflg, i;
> const expressionS *t = &emptyE;
>
> pr_debug ("%s:%d: find_opcode_match: trying opcode 0x%08X ",
> @@ -1596,20 +1596,23 @@ find_opcode_match (const struct arc_opcode
> *first_opcode,
> }
> pr_debug ("opr ");
>
> - /* Check the flags. Iterate over the valid flag classes. */
> - int lnflg = nflgs;
> + /* Setup ready for flag parsing. */
> + lnflg = nflgs;
> + for (i = 0; i < nflgs; i++)
> + first_pflag [i].code = 0;
>
> - for (flgidx = opcode->flags; *flgidx && lnflg; ++flgidx)
> + /* Check the flags. Iterate over the valid flag classes. */
> + for (flgidx = opcode->flags; *flgidx; ++flgidx)
> {
> /* Get a valid flag class. */
> const struct arc_flag_class *cl_flags = &arc_flag_classes[*flgidx];
> const unsigned *flgopridx;
> + int cl_matches = 0;
>
> for (flgopridx = cl_flags->flags; *flgopridx; ++flgopridx)
> {
> const struct arc_flag_operand *flg_operand;
> struct arc_flags *pflag = first_pflag;
> - int i;
>
> flg_operand = &arc_flag_operands[*flgopridx];
> for (i = 0; i < nflgs; i++, pflag++)
> @@ -1617,13 +1620,20 @@ find_opcode_match (const struct arc_opcode
> *first_opcode,
> /* Match against the parsed flags. */
> if (!strcmp (flg_operand->name, pflag->name))
> {
> - /*TODO: Check if it is duplicated. */
> + if (pflag->code != 0)
> + goto match_failed;
> + cl_matches++;
> pflag->code = *flgopridx;
> lnflg--;
> break; /* goto next flag class and parsed flag. */
> }
> }
> }
> +
> + if (cl_flags->class == F_CLASS_REQUIRED && cl_matches == 0)
> + goto match_failed;
> + if (cl_flags->class == F_CLASS_OPTIONAL && cl_matches > 1)
> + goto match_failed;
> }
> /* Did I check all the parsed flags? */
> if (lnflg)
> diff --git a/gas/testsuite/gas/arc/asm-errors.d b/gas/testsuite/gas/arc/asm-
> errors.d
> new file mode 100644
> index 0000000..9255a27
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/asm-errors.d
> @@ -0,0 +1,2 @@
> +#as: -mcpu=arc700
> +#error-output: asm-errors.err
> diff --git a/gas/testsuite/gas/arc/asm-errors.err
> b/gas/testsuite/gas/arc/asm-errors.err
> new file mode 100644
> index 0000000..35390fc
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/asm-errors.err
> @@ -0,0 +1,4 @@
> +[^:]*: Assembler messages:
> +[^:]*:2: Error: inappropriate arguments for opcode 'adc'
> +[^:]*:3: Error: inappropriate arguments for opcode 'adc'
> +[^:]*:4: Error: inappropriate arguments for opcode 'adc'
> diff --git a/gas/testsuite/gas/arc/asm-errors.s b/gas/testsuite/gas/arc/asm-
> errors.s
> new file mode 100644
> index 0000000..6e0fd6a
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/asm-errors.s
> @@ -0,0 +1,4 @@
> + .text
> + adc.al.ra r0,r0,r2
> + adc.eq.eq r0,r0,r2
> + adc.n.eq r0,r0,r2
> diff --git a/include/ChangeLog b/include/ChangeLog
> index d531748..7742efa 100644
> --- a/include/ChangeLog
> +++ b/include/ChangeLog
> @@ -1,5 +1,10 @@
> 2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
>
> + * opcode/arc.h (flag_class_t): Remove all old flag classes, add 3
> + new classes instead.
> +
> +2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
> +
> * elf/arc.h (E_ARC_MACH_NPS400): Define.
> * opcode/arc.h (ARC_OPCODE_NPS400): Define.
>
> diff --git a/include/opcode/arc.h b/include/opcode/arc.h
> index 85ea735..3310c10 100644
> --- a/include/opcode/arc.h
> +++ b/include/opcode/arc.h
> @@ -72,15 +72,15 @@ typedef enum
> /* Flags class. */
> typedef enum
> {
> - FNONE,
> - CND, /* Conditional flags. */
> - WBM, /* Write-back modes. */
> - FLG, /* F Flag. */
> - SBP, /* Static branch prediction. */
> - DLY, /* Delay slot. */
> - DIF, /* Bypass caches. */
> - SGX, /* Sign extend modes. */
> - SZM /* Data size modes. */
> + F_CLASS_NONE,
> +
> + /* At most one flag from the set of flags can appear in the
> + instruction. */
> + F_CLASS_OPTIONAL,
> +
> + /* Exactly one from from the set of flags must appear in the
> + instruction. */
> + F_CLASS_REQUIRED,
> } flag_class_t;
>
> /* The opcode table is an array of struct arc_opcode. */
> diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
> index f9c5f77..f590a43 100644
> --- a/opcodes/ChangeLog
> +++ b/opcodes/ChangeLog
> @@ -1,5 +1,10 @@
> 2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
>
> + * arc-opc.c (arc_flag_classes): Convert all flag classes to use
> + the new class enum values.
> +
> +2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
> +
> * arc-dis.c (print_insn_arc): Handle nps400.
>
> 2016-03-15 Andrew Burgess <andrew.burgess@embecosm.com>
> diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
> index a4fdaff..f126fa8 100644
> --- a/opcodes/arc-opc.c
> +++ b/opcodes/arc-opc.c
> @@ -803,65 +803,66 @@ const unsigned arc_num_flag_operands =
> ARRAY_SIZE (arc_flag_operands);
> const struct arc_flag_class arc_flag_classes[] =
> {
> #define C_EMPTY 0
> - { FNONE, { F_NULL } },
> + { F_CLASS_NONE, { F_NULL } },
>
> #define C_CC (C_EMPTY + 1)
> - { CND, { F_ALWAYS, F_RA, F_EQUAL, F_ZERO, F_NOTEQUAL, F_NOTZERO,
> - F_POZITIVE, F_PL, F_NEGATIVE, F_MINUS, F_CARRY, F_CARRYSET,
> - F_LOWER, F_CARRYCLR, F_NOTCARRY, F_HIGHER,
> F_OVERFLOWSET,
> - F_OVERFLOW, F_NOTOVERFLOW, F_OVERFLOWCLR, F_GT, F_GE,
> F_LT,
> - F_LE, F_HI, F_LS, F_PNZ, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_ALWAYS, F_RA, F_EQUAL, F_ZERO,
> F_NOTEQUAL,
> + F_NOTZERO, F_POZITIVE, F_PL, F_NEGATIVE,
> F_MINUS,
> + F_CARRY, F_CARRYSET, F_LOWER, F_CARRYCLR,
> + F_NOTCARRY, F_HIGHER, F_OVERFLOWSET,
> F_OVERFLOW,
> + F_NOTOVERFLOW, F_OVERFLOWCLR, F_GT, F_GE,
> F_LT,
> + F_LE, F_HI, F_LS, F_PNZ, F_NULL } },
>
> #define C_AA_ADDR3 (C_CC + 1)
> #define C_AA27 (C_CC + 1)
> - { WBM, { F_A3, F_AW3, F_AB3, F_AS3, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_A3, F_AW3, F_AB3, F_AS3, F_NULL } },
> #define C_AA_ADDR9 (C_AA_ADDR3 + 1)
> #define C_AA21 (C_AA_ADDR3 + 1)
> - { WBM, { F_A9, F_AW9, F_AB9, F_AS9, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_A9, F_AW9, F_AB9, F_AS9, F_NULL } },
> #define C_AA_ADDR22 (C_AA_ADDR9 + 1)
> #define C_AA8 (C_AA_ADDR9 + 1)
> - { WBM, { F_A22, F_AW22, F_AB22, F_AS22, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_A22, F_AW22, F_AB22, F_AS22, F_NULL } },
>
> #define C_F (C_AA_ADDR22 + 1)
> - { FLG, { F_FLAG, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_FLAG, F_NULL } },
> #define C_FHARD (C_F + 1)
> - { FLG, { F_FFAKE, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_FFAKE, F_NULL } },
>
> #define C_T (C_FHARD + 1)
> - { SBP, { F_NT, F_T, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_NT, F_T, F_NULL } },
> #define C_D (C_T + 1)
> - { DLY, { F_ND, F_D, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_ND, F_D, F_NULL } },
>
> #define C_DHARD (C_D + 1)
> - { DLY, { F_DFAKE, F_NULL } },
> + { F_CLASS_OPTIONAL, { F_DFAKE, F_NULL } },
>
> #define C_DI20 (C_DHARD + 1)
> - { DIF, { F_DI11, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_DI11, F_NULL }},
> #define C_DI16 (C_DI20 + 1)
> - { DIF, { F_DI15, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_DI15, F_NULL }},
> #define C_DI26 (C_DI16 + 1)
> - { DIF, { F_DI5, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_DI5, F_NULL }},
>
> #define C_X25 (C_DI26 + 1)
> - { SGX, { F_SIGN6, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_SIGN6, F_NULL }},
> #define C_X15 (C_X25 + 1)
> - { SGX, { F_SIGN16, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_SIGN16, F_NULL }},
> #define C_XHARD (C_X15 + 1)
> #define C_X (C_X15 + 1)
> - { SGX, { F_SIGNX, F_NULL }},
> + { F_CLASS_OPTIONAL, { F_SIGNX, F_NULL }},
>
> #define C_ZZ13 (C_X + 1)
> - { SZM, { F_SIZEB17, F_SIZEW17, F_H17, F_NULL}},
> + { F_CLASS_OPTIONAL, { F_SIZEB17, F_SIZEW17, F_H17, F_NULL}},
> #define C_ZZ23 (C_ZZ13 + 1)
> - { SZM, { F_SIZEB7, F_SIZEW7, F_H7, F_NULL}},
> + { F_CLASS_OPTIONAL, { F_SIZEB7, F_SIZEW7, F_H7, F_NULL}},
> #define C_ZZ29 (C_ZZ23 + 1)
> - { SZM, { F_SIZEB1, F_SIZEW1, F_H1, F_NULL}},
> + { F_CLASS_OPTIONAL, { F_SIZEB1, F_SIZEW1, F_H1, F_NULL}},
>
> #define C_AS (C_ZZ29 + 1)
> - { SZM, { F_ASFAKE, F_NULL}},
> + { F_CLASS_OPTIONAL, { F_ASFAKE, F_NULL}},
>
> #define C_NE (C_AS + 1)
> - { CND, { F_NE, F_NULL}},
> + { F_CLASS_OPTIONAL, { F_NE, F_NULL}},
> };
>
> /* The operands table.
> --
> 2.5.1