Changes To Fixup Saving, Restoring and Swapping

Changes To Fixup Saving, Restoring and Swapping

[John Healy]

Hi,

A port I'm working on requires packing a set of instructions (two at
this point, more later) into a vliw word.   In order to be able to
handle more than two instructions at a time during assembly, an
enhancement to the gas_cgen_[save/restore/swap]_fixups mechanism is
required. Currently this mechanism allows for one set of fixups to be
store then restored to/swapped with the current set.   I've made some
modifications so that fixup chains for many instructions can be stored
and selected from for restores and swaps.  Just in case it might be of
use, I've also added a little extra feature that allows for the current
fixups to be either kept or cleared when stored and for a set of stored
fixups to be either kept or cleared when restored.   I threw some
documentation in too.

Everything is still done essentially the way it was originally, only the
operations are preformed on the elements of an array, each of which
stores a fixup chain and a fixup count.

The only port that is affected by the changes is m32r .  I updated
tc-m32r.c to reflect the new calling inteface and ran the gas
testsuite.  There are no regressions.

The patch is attached.  Let me know if there are any comments or
objections. If there are none, I'll check it in in a day or so.

Thanks,

John Healy
Simulation Engineer
Red Hat
jhealy@redhat.com

Re: Changes To Fixup Saving, Restoring and Swapping

[Frank Ch. Eigler]

Hi -

jhealy wrote:
: A port I'm working on requires packing a set of instructions (two at
: this point, more later) into a vliw word.   In order to be able to
: handle more than two instructions at a time during assembly, an
: enhancement to the gas_cgen_[save/restore/swap]_fixups mechanism is
: required. [...]

All okay, except for:

: [...]  Just in case it might be of
: use, I've also added a little extra feature that allows for the current
: fixups to be either kept or cleared when stored and for a set of stored
: fixups to be either kept or cleared when restored.   

I would prefer not to have this extension in the code, unless your current
internal port has use for it, or unless you can make a more persuasive
argument for it than "just in case".


: I threw some documentation in too. [and tested on m32r]

Thanks!


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE7RJxeVZbdDOm/ZT0RAsz/AKCH1id0kVDdKAmHx2dWez4hoA23ewCePn8+
l/y/Z+aJm3Ltz9qclyMBCvM=
=f50m
-----END PGP SIGNATURE-----

Changes To Fixup Saving, Restoring and Swapping

[Doug Evans]

Nit: function names go in column 1.

John Healy writes:

 > ! void gas_cgen_initialize_saved_fixups_array()

 > ! void gas_cgen_save_fixups (int i, int current)

 > ! void gas_cgen_restore_fixups(int i, int stored)

 > ! void gas_cgen_swap_fixups(int i)

Re: Changes To Fixup Saving, Restoring and Swapping

[John Healy]

I don't have any particular use for it myself at this point.  The idea was
just that I didn't see the necessity of wiping out the local/saved copy of
fixups that have just been saved/restored so I chose to give the user a
choice.  I can remove it if you like.  Just out of curiosity though, why
don't you want it in there?  Do you see it possibly allowing something
detrimental to happen?

John

"Frank Ch. Eigler" wrote:

> Hi -
>
> jhealy wrote:
> : A port I'm working on requires packing a set of instructions (two at
> : this point, more later) into a vliw word.   In order to be able to
> : handle more than two instructions at a time during assembly, an
> : enhancement to the gas_cgen_[save/restore/swap]_fixups mechanism is
> : required. [...]
>
> All okay, except for:
>
> : [...]  Just in case it might be of
> : use, I've also added a little extra feature that allows for the current
> : fixups to be either kept or cleared when stored and for a set of stored
> : fixups to be either kept or cleared when restored.
>
> I would prefer not to have this extension in the code, unless your current
> internal port has use for it, or unless you can make a more persuasive
> argument for it than "just in case".
>
> : I threw some documentation in too. [and tested on m32r]
>
> Thanks!
>
> - FChE
>
>   ------------------------------------------------------------------------
>    Part 1.2Type: application/pgp-signature

Re: Changes To Fixup Saving, Restoring and Swapping

[Frank Ch. Eigler]

Hi -

jhealy wrote:
: I don't have any particular use for [the KEEP_* options] myself
: at this point.  [...]  Just out of curiosity though, why don't
: you want it in there?  [...]

It complicates the code, and pollutes the name space a little bit.
More importantly, it's an option that a new program will be forced
to spend time trying to understand and avoid.  Until it's shown
to be helpful, I would like to not even have to consider it.
(If you like, leave a comment behind in the code, as a reminder.)

It's like seeing forty five types of green salad on a restaurant
menu, when the hungry masses all like the same one.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE7RKEsVZbdDOm/ZT0RAig7AJ9gp3DpeyM2VPPYUz8g86X5G+6XoACfQiL7
VQlHNPqZuUXs9phYt+muaT8=
=+FrD
-----END PGP SIGNATURE-----

Re: Changes To Fixup Saving, Restoring and Swapping

[John Healy]

"Frank Ch. Eigler" wrote:

> It complicates the code, and pollutes the name space a little bit.
> More importantly, it's an option that a new program will be forced
> to spend time trying to understand and avoid.  Until it's shown
> to be helpful, I would like to not even have to consider it.
> (If you like, leave a comment behind in the code, as a reminder.)

Good points.  'Tis gone.   I've also put the function names in column 1 as
Doug suggested.   The new patch is attached.  It builds and tests fine.  No
regressions.

> It's like seeing forty five types of green salad on a restaurant
> menu, when the hungry masses all like the same one.

If the masses are that hungry, I would suggest a cheeseburger from Mars Diner
instead of a salad - unless, of course, one of those forty-five types of
salad is a Cheeseburger Salad.  Hey, what a great idea!  Don't forget folks
...... you saw it here first!  :P

John

Re: Changes To Fixup Saving, Restoring and Swapping

[Greg McGary]

Doug Evans <dje@transmeta.com> writes:

> Nit: function names go in column 1.

'Nother Nit: A space goes between function name and open paren.

> John Healy writes:
> 
>  > ! void gas_cgen_initialize_saved_fixups_array()
> 
>  > ! void gas_cgen_save_fixups (int i, int current)
> 
>  > ! void gas_cgen_restore_fixups(int i, int stored)
> 
>  > ! void gas_cgen_swap_fixups(int i)

Re: Changes To Fixup Saving, Restoring and Swapping

[John Healy]

Right.  Fixed.   Sloppy me.

Thanks,

John

Greg McGary wrote:

> Doug Evans <dje@transmeta.com> writes:
>
> > Nit: function names go in column 1.
>
> 'Nother Nit: A space goes between function name and open paren.
>
> > John Healy writes:
> >
> >  > ! void gas_cgen_initialize_saved_fixups_array()
> >
> >  > ! void gas_cgen_save_fixups (int i, int current)
> >
> >  > ! void gas_cgen_restore_fixups(int i, int stored)
> >
> >  > ! void gas_cgen_swap_fixups(int i)

Re: Changes To Fixup Saving, Restoring and Swapping

[John Healy]

Committed.

John


>
>
>   ------------------------------------------------------------------------
> [gas]
>
> 2001-07-05  John Healy  <jhealy@redhat.com>
>
>         * cgen.c (gas_cgen_save_fixups): Modified to allow more than one
>         set of fixups to be stored.
>         (gas_cgen_restore_fixups): Modified to allow the fixup chain to be
>         restored to be chosen from any that are saved.
>         (gas_cgen_swap_fixups): Modified to allow the current set of
>         fixups to be swapped with any other set that has been saved.
>         (gas_cgen_initialize_saved_fixups_array): New routine.
>         * cgen.h: Modifed prototypes for gas_cgen_save_fixups,
>         gas_cgen_restore_fixups, and gas_cgen_swap_fixups.  Added definitions
>         for MAX_SAVED_FIXUP_CHAINS.
>         * config/tc-m32r.c (assemble_two_insns): Changed calls to fixup
>         store, swap and restore fuctions to reflect the new interface.
>
>   ------------------------------------------------------------------------
> Index: gas/cgen.c
> ===================================================================
> RCS file: /cvs/src/src/gas/cgen.c,v
> retrieving revision 1.12
> diff -c -3 -p -r1.12 cgen.c
> *** cgen.c      2001/05/16 23:06:02     1.12
> --- cgen.c      2001/07/05 17:24:01
> *************** queue_fixup (opindex, opinfo, expP)
> *** 94,155 ****
>     ++ num_fixups;
>   }
>
> ! /* The following three functions allow a backup of the fixup chain to be made,
> !    and to have this backup be swapped with the current chain.  This allows
> !    certain ports, eg the m32r, to swap two instructions and swap their fixups
> !    at the same time.  */
> ! /* ??? I think with cgen_asm_finish_insn (or something else) there is no
> !    more need for this.  */
>
> ! static struct fixup saved_fixups[GAS_CGEN_MAX_FIXUPS];
> ! static int saved_num_fixups;
>
> ! void
> ! gas_cgen_save_fixups ()
>   {
> !   saved_num_fixups = num_fixups;
> !
> !   memcpy (saved_fixups, fixups, sizeof (fixups[0]) * num_fixups);
> !
> !   num_fixups = 0;
>   }
>
> ! void
> ! gas_cgen_restore_fixups ()
>   {
> !   num_fixups = saved_num_fixups;
> !
> !   memcpy (fixups, saved_fixups, sizeof (fixups[0]) * num_fixups);
>
> !   saved_num_fixups = 0;
>   }
>
> ! void
> ! gas_cgen_swap_fixups ()
>   {
> !   int tmp;
> !   struct fixup tmp_fixup;
>
> !   if (num_fixups == 0)
> !     {
> !       gas_cgen_restore_fixups ();
> !     }
> !   else if (saved_num_fixups == 0)
> !     {
> !       gas_cgen_save_fixups ();
> !     }
> !   else
> !     {
> !       tmp = saved_num_fixups;
> !       saved_num_fixups = num_fixups;
> !       num_fixups = tmp;
> !
> !       for (tmp = GAS_CGEN_MAX_FIXUPS; tmp--;)
> !       {
> !         tmp_fixup          = saved_fixups [tmp];
> !         saved_fixups [tmp] = fixups [tmp];
> !         fixups [tmp]       = tmp_fixup;
> !       }
>       }
>   }
>
> --- 94,208 ----
>     ++ num_fixups;
>   }
>
> ! /* The following functions allow fixup chains to be stored, retrieved,
> !    and swapped.  They are a generalization of a pre-existing scheme
> !    for storing, restoring and swapping fixup chains that was used by
> !    the m32r port.  The functionality is essentially the same, only
> !    instead of only being able to store a single fixup chain, an entire
> !    array of fixup chains can be stored.  It is the user's responsibility
> !    to keep track of how many fixup chains have been stored and which
> !    elements of the array they are in.
> !
> !    The algorithms used are the same as in the old scheme.  Other than the
> !    "array-ness" of the whole thing, the functionality is identical to the
> !    old scheme.
> !
> !    gas_cgen_initialize_saved_fixups_array():
> !       Sets num_fixups_in_chain to 0 for each element. Call this from
> !       md_begin() if you plan to use these functions and you want the
> !       fixup count in each element to be set to 0 intially.  This is
> !       not necessary, but it's included just in case.  It performs
> !       the same function for each element in the array of fixup chains
> !       that gas_init_parse() performs for the current fixups.
> !
> !    gas_cgen_save_fixups (element):
> !       element - element number of the array you wish to store the fixups
> !                 to.  No mechanism is built in for tracking what element
> !                 was last stored to.
> !
> !    gas_cgen_restore_fixups (element):
> !       element - element number of the array you wish to restore the fixups
> !                 from.
> !
> !    gas_cgen_swap_fixups(int element):
> !        element - swap the current fixups with those in this element number.
> ! */
> !
> ! struct saved_fixups {
> !      struct fixup fixup_chain[GAS_CGEN_MAX_FIXUPS];
> !      int num_fixups_in_chain;
> ! };
>
> ! static struct saved_fixups stored_fixups[MAX_SAVED_FIXUP_CHAINS];
>
> ! void
> ! gas_cgen_initialize_saved_fixups_array()
>   {
> !    int i = 0;
> !    while (i < MAX_SAVED_FIXUP_CHAINS)
> !       stored_fixups[i++].num_fixups_in_chain = 0;
>   }
>
> ! void
> ! gas_cgen_save_fixups (int i)
>   {
> !       if (i < 0 || i >= MAX_SAVED_FIXUP_CHAINS)
> !       {
> !           as_fatal("Index into stored_fixups[] out of bounds.");
> !           return;
> !       }
> !       stored_fixups[i].num_fixups_in_chain = num_fixups;
> !       memcpy(stored_fixups[i].fixup_chain, fixups,
> !                     sizeof (fixups[0])*num_fixups);
> !       num_fixups = 0;
> ! }
>
> ! void
> ! gas_cgen_restore_fixups(int i)
> ! {
> !       if (i < 0 || i >= MAX_SAVED_FIXUP_CHAINS)
> !       {
> !           as_fatal("Index into stored_fixups[] out of bounds.");
> !           return;
> !       }
> !       num_fixups = stored_fixups[i].num_fixups_in_chain;
> !       memcpy(fixups,stored_fixups[i].fixup_chain,
> !                     (sizeof (stored_fixups[i].fixup_chain[0]))*num_fixups);
> !       stored_fixups[i].num_fixups_in_chain = 0;
>   }
>
> ! void
> ! gas_cgen_swap_fixups(int i)
>   {
> !      int tmp;
> !      struct fixup tmp_fixup;
>
> !      if (i < 0 || i >= MAX_SAVED_FIXUP_CHAINS)
> !      {
> !          as_fatal("Index into stored_fixups[] out of bounds.");
> !          return;
> !      }
> !
> !      if (num_fixups == 0)
> !      {
> !        gas_cgen_restore_fixups (i);
> !      }
> !      else if (stored_fixups[i].num_fixups_in_chain == 0)
> !      {
> !        gas_cgen_save_fixups (i);
> !      }
> !      else
> !      {
> !        tmp = stored_fixups[i].num_fixups_in_chain;
> !        stored_fixups[i].num_fixups_in_chain = num_fixups;
> !        num_fixups = tmp;
> !
> !        for (tmp = GAS_CGEN_MAX_FIXUPS; tmp--;)
> !        {
> !          tmp_fixup          = stored_fixups[i].fixup_chain [tmp];
> !          stored_fixups[i].fixup_chain[tmp] = fixups [tmp];
> !          fixups [tmp]       = tmp_fixup;
> !        }
>       }
>   }
>
> Index: gas/cgen.h
> ===================================================================
> RCS file: /cvs/src/src/gas/cgen.h,v
> retrieving revision 1.7
> diff -c -3 -p -r1.7 cgen.h
> *** cgen.h      2001/03/08 23:24:22     1.7
> --- cgen.h      2001/07/05 17:24:01
> *************** extern const char * gas_cgen_parse_opera
> *** 53,61 ****
>   /* Call this from md_assemble to initialize the assembler callback.  */
>   extern void gas_cgen_init_parse PARAMS ((void));
>
> ! extern void gas_cgen_save_fixups PARAMS ((void));
> ! extern void gas_cgen_restore_fixups PARAMS ((void));
> ! extern void gas_cgen_swap_fixups PARAMS ((void));
>
>   /* Add a register to the assembler's hash table.
>      This makes lets GAS parse registers for us.
> --- 53,64 ----
>   /* Call this from md_assemble to initialize the assembler callback.  */
>   extern void gas_cgen_init_parse PARAMS ((void));
>
> ! /* Routines and macros for saving fixup chains. */
> ! extern void gas_cgen_save_fixups PARAMS ((int));
> ! extern void gas_cgen_restore_fixups PARAMS ((int));
> ! extern void gas_cgen_swap_fixups PARAMS ((int));
> ! extern void gas_cgen_initialize_saved_fixups_array PARAMS ((void));
> ! #define MAX_SAVED_FIXUP_CHAINS 50
>
>   /* Add a register to the assembler's hash table.
>      This makes lets GAS parse registers for us.
> Index: gas/config/tc-m32r.c
> ===================================================================
> RCS file: /cvs/src/src/gas/config/tc-m32r.c,v
> retrieving revision 1.19
> diff -c -3 -p -r1.19 tc-m32r.c
> *** tc-m32r.c   2001/05/10 11:32:51     1.19
> --- tc-m32r.c   2001/07/05 17:24:02
> *************** md_begin ()
> *** 554,559 ****
> --- 554,561 ----
>     scom_symbol.section         = &scom_section;
>
>     allow_m32rx (enable_m32rx);
> +
> +   gas_cgen_initialize_saved_fixups_array();
>   }
>
>   #define OPERAND_IS_COND_BIT(operand, indices, index) \
> *************** assemble_two_insns (str, str2, parallel_
> *** 832,838 ****
>
>     /* Preserve any fixups that have been generated and reset the list
>        to empty.  */
> !   gas_cgen_save_fixups ();
>
>     /* Get the indices of the operands of the instruction.  */
>     /* FIXME: CGEN_FIELDS is already recorded, but relying on that fact
> --- 834,840 ----
>
>     /* Preserve any fixups that have been generated and reset the list
>        to empty.  */
> !   gas_cgen_save_fixups (0);
>
>     /* Get the indices of the operands of the instruction.  */
>     /* FIXME: CGEN_FIELDS is already recorded, but relying on that fact
> *************** assemble_two_insns (str, str2, parallel_
> *** 941,947 ****
>         || (errmsg = (char *) can_make_parallel (&first, &second)) == NULL)
>       {
>         /* Get the fixups for the first instruction.  */
> !       gas_cgen_swap_fixups ();
>
>         /* Write it out.  */
>         expand_debug_syms (first.debug_sym_link, 1);
> --- 943,949 ----
>         || (errmsg = (char *) can_make_parallel (&first, &second)) == NULL)
>       {
>         /* Get the fixups for the first instruction.  */
> !       gas_cgen_swap_fixups (0);
>
>         /* Write it out.  */
>         expand_debug_syms (first.debug_sym_link, 1);
> *************** assemble_two_insns (str, str2, parallel_
> *** 953,959 ****
>         make_parallel (second.buffer);
>
>         /* Get its fixups.  */
> !       gas_cgen_restore_fixups ();
>
>         /* Write it out.  */
>         expand_debug_syms (second.debug_sym_link, 1);
> --- 955,961 ----
>         make_parallel (second.buffer);
>
>         /* Get its fixups.  */
> !       gas_cgen_restore_fixups (0);
>
>         /* Write it out.  */
>         expand_debug_syms (second.debug_sym_link, 1);
> *************** assemble_two_insns (str, str2, parallel_
> *** 972,978 ****
>         make_parallel (first.buffer);
>
>         /* Get the fixups for the first instruction.  */
> !       gas_cgen_restore_fixups ();
>
>         /* Write out the first instruction.  */
>         expand_debug_syms (first.debug_sym_link, 1);
> --- 974,980 ----
>         make_parallel (first.buffer);
>
>         /* Get the fixups for the first instruction.  */
> !       gas_cgen_restore_fixups (0);
>
>         /* Write out the first instruction.  */
>         expand_debug_syms (first.debug_sym_link, 1);