Patchwork RFA: Add lock_lenth attribute to support the ARC port (Was: Re: Ping: RFA: add lock_length attribute to break branch-shortening cycles)

login
register
mail settings
Submitter Joern Rennecke
Date Oct. 24, 2012, 1:42 a.m.
Message ID <20121023214204.vsnfi46iyscw4sos-nzlynne@webmail.spamcop.net>
Download mbox | patch
Permalink /patch/193614/
State New
Headers show

Comments

Joern Rennecke - Oct. 24, 2012, 1:42 a.m.
Quoting Richard Biener <richard.guenther@gmail.com>:

> On Tue, Oct 16, 2012 at 9:35 PM, Joern Rennecke
> <joern.rennecke@embecosm.com> wrote:
..
>> Well, we could split it anyway, and give ports without the need for
>> multiple length attributes the benefit of the optimistic algorithm.
>>
>> I have attached a patch that implements this.
>
> Looks reasonable to me, though I'm not familiar enough with the code
> to approve it.

Now that Richard Sandiford has reviewed that split-off part and it's in
the source tree, we can return to the remaining functionality needed
by for the ARC port.

> I'd strongly suggest to try harder to make things work for you without
> the new attribute even though I wasn't really able to follow your reasoning
> on why that wouldn't work.  It may be easier to motivate this change
> once the port is in without that attribute so one can actually look at
> the machine description and port details.

Well, it doesn't simply drop in with the existing branch shortening -
libgcc won't compile because of out-of-range branches.
I tried to lump the length and lock_length atribute together, and that
just gives genattrtab indigestion.  It sits there looping forever.
I could start debugging this, but that would take an unknown amount of
time, and then the review of the fix would take an unknown amount of time,
and then the ARC port probably needs fixing up again because it just
doesn't work right with these fudged lengths.  And even if we could get
everything required in before the close of phase 1, the branch shortening
would be substandard.
It seems more productive to get the branch shortening working now.
The lock_length atrtibute is completely optional, so no port maintainer
would be forced to use the functionality if it's not desired.

The issue is that the some instructions need to be aligned or unaligned
for performance or in a few cases even for correctness.  Just inserting
random nops would be a waste of code space and cycles, since most of the
time, the desired (mis)alignment can be archived by selectively making
a short instruction long.  If an instruction that is long once was forced
to stay long henceforth, that would actually defeat the purpose of getting
the desired alignment.  Then another short instruction - if it can be found -
would need to be upsized.  So a size increase could ripple through as
alignments are distorted.  The natural thing to do is really when the
alignemnt changes is really to let the upsized instruction be short again.
Only length-determined branch sizes have to be locked to avoid cycles.

This is the documentation for the new role of the lock_length attribute
(reduced from my previous attempt):

@cindex lock_length
Usually, when doing optimizing branch shortening, the instruction length
is calculated by avaluating the @code{length} attribute, applying
@code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
value and the length of the instruction in the previous iteration.

If you define the @code{lock_length} attribute, the @code{lock_length}
attribute will be evaluated, and then the maximum with of @code{lock_length}
value from the previous iteration will be formed and saved.
Then the maximum of that value with the @code{length} attribute will
be formed, and @code{ADJUST_INSN_LENGTH} will be applied.
Thus, you can allow the length to vary downwards as well as upwards
across iterations with suitable definitions of the @code{length} attribute
and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
lead to infinite loops.

The new patch builds on this patch:
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01890.html
as a prerequisite.

build tested with libraries in revision 192654 for i686-pc-linux-gnu X  
mipsel-elf .
bootstrapped in revision 192703 on i686-pc-linux-gnu;
I've also successfully run config-list.mk with the patch applied to this
revision.  The following ports had pre-existing failures, which are
documented in the sub-PRs or PR47093/PR44756:

alpha64-dec-vms alpha-dec-vms am33_2.0-linux arm-netbsdelf arm-wrs-vxworks
avr-elf avr-rtems c6x-elf c6x-uclinux cr16-elf fr30-elf
i686-interix3 --enable-obsolete i686-openbsd3.0 i686-pc-msdosdjgpp
ia64-hp-vms iq2000-elf lm32-elf lm32-rtems lm32-uclinux mep-elf
microblaze-elf microblaze-linux mn10300-elf moxie-elf moxie-rtems
moxie-uclinux pdp11-aout picochip-elf --enable-obsolete rl78-elf
rx-elf score-elf --enable-obsolete sh5el-netbsd sh64-elf --with-newlib
sh64-linux sh64-netbsd sh-elf shle-linux sh-netbsdelf sh-rtems sh-superh-elf
sh-wrs-vxworks tilegx-linux-gnu tilepro-linux-gnu vax-linux-gnu vax-netbsdelf
vax-openbsd x86_64-knetbsd-gnu


I'll be posting the ARC port shortly; it does not fit into a single  
100 KB posting, so I'm thinking of splitting it in a configury patch  
and zx
compressed files/tarballs for arc.c, arc.md, libgcc, and the rest of the port.
2012-10-22  Joern Rennecke  <joern.rennecke@embecosm.com>

	* doc/md.texi (node Defining Attributes): Add lock_length to
	table of special attributes.
	(node Insn Lengths): Document lock_length attribute.
	* final.c (uid_lock_length): New variable.
	(insn_max_length, get_attr_lock_length): New functions.
	(get_attr_length): Use insn_max_length.
	(get_attr_length_1): Use direct recursion rather than
	calling get_attr_length.
	(INSN_VARIABLE_LENGTH_P): Define.
	(shorten_branches): Implement lock_length attribute functionality.
	* genattrtab.c (lock_length_str): New variable.
	(make_length_attrs): New parameter base.
	(main): Initialize lock_length_str.
	Generate lock_lengths attributes.
	* genattr.c (gen_attr): Emit declarations for lock_length attribute
	related functions.
	(main): Emit stub defines for lock_length attribute related functions.
Richard Guenther - Oct. 24, 2012, 10:35 a.m.
On Wed, Oct 24, 2012 at 3:42 AM, Joern Rennecke
<joern.rennecke@embecosm.com> wrote:
> Quoting Richard Biener <richard.guenther@gmail.com>:
>
>> On Tue, Oct 16, 2012 at 9:35 PM, Joern Rennecke
>> <joern.rennecke@embecosm.com> wrote:
>
> ..
>>>
>>> Well, we could split it anyway, and give ports without the need for
>>> multiple length attributes the benefit of the optimistic algorithm.
>>>
>>> I have attached a patch that implements this.
>>
>>
>> Looks reasonable to me, though I'm not familiar enough with the code
>> to approve it.
>
>
> Now that Richard Sandiford has reviewed that split-off part and it's in
> the source tree, we can return to the remaining functionality needed
> by for the ARC port.
>
>> I'd strongly suggest to try harder to make things work for you without
>> the new attribute even though I wasn't really able to follow your
>> reasoning
>> on why that wouldn't work.  It may be easier to motivate this change
>> once the port is in without that attribute so one can actually look at
>> the machine description and port details.
>
>
> Well, it doesn't simply drop in with the existing branch shortening -
> libgcc won't compile because of out-of-range branches.
> I tried to lump the length and lock_length atribute together, and that
> just gives genattrtab indigestion.  It sits there looping forever.
> I could start debugging this, but that would take an unknown amount of
> time, and then the review of the fix would take an unknown amount of time,
> and then the ARC port probably needs fixing up again because it just
> doesn't work right with these fudged lengths.  And even if we could get
> everything required in before the close of phase 1, the branch shortening
> would be substandard.
> It seems more productive to get the branch shortening working now.
> The lock_length atrtibute is completely optional, so no port maintainer
> would be forced to use the functionality if it's not desired.
>
> The issue is that the some instructions need to be aligned or unaligned
> for performance or in a few cases even for correctness.  Just inserting
> random nops would be a waste of code space and cycles, since most of the
> time, the desired (mis)alignment can be archived by selectively making
> a short instruction long.  If an instruction that is long once was forced
> to stay long henceforth, that would actually defeat the purpose of getting
> the desired alignment.  Then another short instruction - if it can be found
> -
> would need to be upsized.  So a size increase could ripple through as
> alignments are distorted.  The natural thing to do is really when the
> alignemnt changes is really to let the upsized instruction be short again.
> Only length-determined branch sizes have to be locked to avoid cycles.

Just to add some extra information, can you quote your ports
ADJUST_INSN_LENGTH and one example instruction with length/lock_length
attribute the above applies to?

> This is the documentation for the new role of the lock_length attribute
> (reduced from my previous attempt):
>
> @cindex lock_length
> Usually, when doing optimizing branch shortening, the instruction length
> is calculated by avaluating the @code{length} attribute, applying
> @code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
> value and the length of the instruction in the previous iteration.

Which sounds straight-forward.  The docs of ADJUST_INSN_LENGTH
are not entirely clear, but it sounds like it may only increase length, correct?
I see that ADJUST_INSN_LENGTH is really not needed as everything
should be expressable in the length attribute of an instruction?

> If you define the @code{lock_length} attribute, the @code{lock_length}
> attribute will be evaluated, and then the maximum with of @code{lock_length}

with of?  I read it as 'of the'

> value from the previous iteration will be formed and saved.

So lock_length will only increase during iteration.

> Then the maximum of that value with the @code{length} attribute will
> be formed, and @code{ADJUST_INSN_LENGTH} will be applied.

ADJUST_INSN_LENGTH will be applied to the maximum?  What will
be the 'instruction length' equivalent to the simple non-lock_length case?
Is it the above, max (ADJUST_INSN_LENGTH (lock-length-max, length))?

> Thus, you can allow the length to vary downwards as well as upwards
> across iterations with suitable definitions of the @code{length} attribute
> and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
> lead to infinite loops.

I don't see that you can shrink length with just suitable lock_length and
length attributes.  What seems to be the cruical difference is that you
apply ADJUST_INSN_LENGTH after combining lock-length-max and length.
But then you _decrease_ length with ADJUST_INSN_LENGHT ...

Maybe what you really want is ADJUST_INSN_LENGTH_AFTER which is
applied afterwards?  Thus,

> Usually, when doing optimizing branch shortening, the instruction length
> is calculated by avaluating the @code{length} attribute, applying
> @code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
> value and the length of the instruction in the previous iteration.
> If @code{ADJUST_INSN_LENGTH_AFTER} is defined it is applied to the
> resulting instruction length.

?

Note that

> Care has to be taken that this does not
> lead to infinite loops.

doesn't convince me that is properly designed ;)

Thanks,
Richard.

>
> The new patch builds on this patch:
> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01890.html
> as a prerequisite.
>
> build tested with libraries in revision 192654 for i686-pc-linux-gnu X
> mipsel-elf .
> bootstrapped in revision 192703 on i686-pc-linux-gnu;
> I've also successfully run config-list.mk with the patch applied to this
> revision.  The following ports had pre-existing failures, which are
> documented in the sub-PRs or PR47093/PR44756:
>
> alpha64-dec-vms alpha-dec-vms am33_2.0-linux arm-netbsdelf arm-wrs-vxworks
> avr-elf avr-rtems c6x-elf c6x-uclinux cr16-elf fr30-elf
> i686-interix3 --enable-obsolete i686-openbsd3.0 i686-pc-msdosdjgpp
> ia64-hp-vms iq2000-elf lm32-elf lm32-rtems lm32-uclinux mep-elf
> microblaze-elf microblaze-linux mn10300-elf moxie-elf moxie-rtems
> moxie-uclinux pdp11-aout picochip-elf --enable-obsolete rl78-elf
> rx-elf score-elf --enable-obsolete sh5el-netbsd sh64-elf --with-newlib
> sh64-linux sh64-netbsd sh-elf shle-linux sh-netbsdelf sh-rtems sh-superh-elf
> sh-wrs-vxworks tilegx-linux-gnu tilepro-linux-gnu vax-linux-gnu
> vax-netbsdelf
> vax-openbsd x86_64-knetbsd-gnu
>
>
> I'll be posting the ARC port shortly; it does not fit into a single 100 KB
> posting, so I'm thinking of splitting it in a configury patch and zx
> compressed files/tarballs for arc.c, arc.md, libgcc, and the rest of the
> port.
>
> 2012-10-22  Joern Rennecke  <joern.rennecke@embecosm.com>
>
>         * doc/md.texi (node Defining Attributes): Add lock_length to
>         table of special attributes.
>         (node Insn Lengths): Document lock_length attribute.
>         * final.c (uid_lock_length): New variable.
>         (insn_max_length, get_attr_lock_length): New functions.
>         (get_attr_length): Use insn_max_length.
>         (get_attr_length_1): Use direct recursion rather than
>         calling get_attr_length.
>         (INSN_VARIABLE_LENGTH_P): Define.
>         (shorten_branches): Implement lock_length attribute functionality.
>         * genattrtab.c (lock_length_str): New variable.
>         (make_length_attrs): New parameter base.
>         (main): Initialize lock_length_str.
>         Generate lock_lengths attributes.
>         * genattr.c (gen_attr): Emit declarations for lock_length attribute
>         related functions.
>         (main): Emit stub defines for lock_length attribute related
> functions.
>
> Index: doc/md.texi
> ===================================================================
> --- doc/md.texi (revision 2660)
> +++ doc/md.texi (working copy)
> @@ -7515,6 +7515,9 @@ extern enum attr_type get_attr_type ();
>  code chunks.  This is especially important when verifying branch
>  distances. @xref{Insn Lengths}.
>
> +@item lock_length
> +The @code{lock_length} attribute.
> +
>  @item enabled
>  The @code{enabled} attribute can be defined to prevent certain
>  alternatives of an insn definition from being used during code
> @@ -8038,6 +8041,22 @@ (define_insn "jump"
>                        (const_int 6)))])
>  @end smallexample
>
> +@cindex lock_length
> +Usually, when doing optimizing branch shortening, the instruction length
> +is calculated by avaluating the @code{length} attribute, applying
> +@code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
> +value and the length of the instruction in the previous iteration.
> +
> +If you define the @code{lock_length} attribute, the @code{lock_length}
> +attribute will be evaluated, and then the maximum with of
> @code{lock_length}
> +value from the previous iteration will be formed and saved.
> +Then the maximum of that value with the @code{length} attribute will
> +be formed, and @code{ADJUST_INSN_LENGTH} will be applied.
> +Thus, you can allow the length to vary downwards as well as upwards
> +across iterations with suitable definitions of the @code{length} attribute
> +and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
> +lead to infinite loops.
> +
>  @end ifset
>  @ifset INTERNALS
>  @node Constant Attributes
> Index: final.c
> ===================================================================
> --- final.c     (revision 2660)
> +++ final.c     (working copy)
> @@ -308,6 +308,7 @@ dbr_sequence_length (void)
>     `insn_current_length'.  */
>
>  static int *insn_lengths;
> +static int *uid_lock_length;
>
>  VEC(int,heap) *insn_addresses_;
>
> @@ -430,14 +431,41 @@ get_attr_length_1 (rtx insn, int (*fallb
>    return length;
>  }
>
> +/* Calculate the maximum length of INSN.  */
> +static int
> +insn_max_length (rtx insn)
> +{
> +  int length, lock_length;
> +
> +  length = insn_default_length (insn);
> +  if (HAVE_ATTR_lock_length)
> +    {
> +      lock_length = insn_default_lock_length (insn);
> +      if (length < lock_length)
> +       length = lock_length;
> +    }
> +  return length;
> +}
> +
>  /* Obtain the current length of an insn.  If branch shortening has been
> done,
>     get its actual length.  Otherwise, get its maximum length.  */
>  int
>  get_attr_length (rtx insn)
>  {
> -  return get_attr_length_1 (insn, insn_default_length);
> +  return get_attr_length_1 (insn, insn_max_length);
>  }
>
> +int
> +get_attr_lock_length (rtx insn)
> +{
> +  if (uid_lock_length && insn_lengths_max_uid > INSN_UID (insn))
> +    return uid_lock_length[INSN_UID (insn)];
> +  return get_attr_length_1 (insn, insn_min_lock_length);
> +}
> +
> +#define INSN_VARIABLE_LENGTH_P(INSN) \
> +  (insn_variable_length_p (INSN) || insn_variable_lock_length_p (INSN))
> +
>  /* Obtain the current length of an insn.  If branch shortening has been
> done,
>     get its actual length.  Otherwise, get its minimum length.  */
>  int
> @@ -966,6 +994,11 @@ shorten_branches (rtx first)
>    /* Allocate the rest of the arrays.  */
>    insn_lengths = XNEWVEC (int, max_uid);
>    insn_lengths_max_uid = max_uid;
> +  if (HAVE_ATTR_lock_length)
> +    uid_lock_length = XCNEWVEC (int, max_uid);
> +  else
> +    uid_lock_length = insn_lengths;
> +
>    /* Syntax errors can lead to labels being outside of the main insn
> stream.
>       Initialize insn_addresses, so that we get reproducible results.  */
>    INSN_ADDRESSES_ALLOC (max_uid);
> @@ -1064,7 +1097,7 @@ shorten_branches (rtx first)
>  #endif /* CASE_VECTOR_SHORTEN_MODE */
>
>    /* Compute initial lengths, addresses, and varying flags for each insn.
> */
> -  int (*length_fun) (rtx) = increasing ? insn_min_length :
> insn_default_length;
> +  int (*length_fun) (rtx) = increasing ? insn_min_length : insn_max_length;
>
>    for (insn_current_address = 0, insn = first;
>         insn != 0;
> @@ -1106,7 +1139,7 @@ shorten_branches (rtx first)
>           /* Alignment is handled by ADDR_VEC_ALIGN.  */
>         }
>        else if (GET_CODE (body) == ASM_INPUT || asm_noperands (body) >= 0)
> -       insn_lengths[uid] = asm_insn_count (body) * insn_default_length
> (insn);
> +       insn_lengths[uid] = asm_insn_count (body) * insn_max_length (insn);
>        else if (GET_CODE (body) == SEQUENCE)
>         {
>           int i;
> @@ -1117,7 +1150,7 @@ shorten_branches (rtx first)
>           const_delay_slots = 0;
>  #endif
>           int (*inner_length_fun) (rtx)
> -           = const_delay_slots ? length_fun : insn_default_length;
> +           = const_delay_slots ? length_fun : insn_max_length;
>           /* Inside a delay slot sequence, we do not do any branch
> shortening
>              if the shortening could change the number of delay slots
>              of the branch.  */
> @@ -1130,7 +1163,7 @@ shorten_branches (rtx first)
>               if (GET_CODE (body) == ASM_INPUT
>                   || asm_noperands (PATTERN (XVECEXP (body, 0, i))) >= 0)
>                 inner_length = (asm_insn_count (PATTERN (inner_insn))
> -                               * insn_default_length (inner_insn));
> +                               * insn_max_length (inner_insn));
>               else
>                 inner_length = inner_length_fun (inner_insn);
>
> @@ -1138,7 +1171,7 @@ shorten_branches (rtx first)
>               if (const_delay_slots)
>                 {
>                   if ((varying_length[inner_uid]
> -                      = insn_variable_length_p (inner_insn)) != 0)
> +                      = INSN_VARIABLE_LENGTH_P (inner_insn)) != 0)
>                     varying_length[uid] = 1;
>                   INSN_ADDRESSES (inner_uid) = (insn_current_address
>                                                 + insn_lengths[uid]);
> @@ -1151,7 +1184,7 @@ shorten_branches (rtx first)
>        else if (GET_CODE (body) != USE && GET_CODE (body) != CLOBBER)
>         {
>           insn_lengths[uid] = length_fun (insn);
> -         varying_length[uid] = insn_variable_length_p (insn);
> +         varying_length[uid] = INSN_VARIABLE_LENGTH_P (insn);
>         }
>
>        /* If needed, do any adjustment.  */
> @@ -1354,7 +1387,7 @@ shorten_branches (rtx first)
>                 {
>                   rtx inner_insn = XVECEXP (body, 0, i);
>                   int inner_uid = INSN_UID (inner_insn);
> -                 int inner_length;
> +                 int inner_length, lock_length;
>
>                   INSN_ADDRESSES (inner_uid) = insn_current_address;
>
> @@ -1365,16 +1398,23 @@ shorten_branches (rtx first)
>                   else
>                     inner_length = insn_current_length (inner_insn);
>
> -                 if (inner_length != insn_lengths[inner_uid])
> +                 lock_length
> +                   = (HAVE_ATTR_lock_length
> +                      ? insn_current_lock_length (inner_insn) :
> inner_length);
> +                 if (lock_length != uid_lock_length[inner_uid])
>                     {
> -                     if (!increasing || inner_length >
> insn_lengths[inner_uid])
> +                     if (!increasing
> +                         || lock_length > uid_lock_length[inner_uid])
>                         {
> -                         insn_lengths[inner_uid] = inner_length;
> +                         uid_lock_length[inner_uid] = lock_length;
>                           something_changed = 1;
>                         }
>                       else
> -                       inner_length = insn_lengths[inner_uid];
> +                       lock_length = uid_lock_length[inner_uid];
>                     }
> +                 if (inner_length < lock_length)
> +                   inner_length = lock_length;
> +                 insn_lengths[inner_uid] = inner_length;
>                   insn_current_address += inner_length;
>                   new_length += inner_length;
>                 }
> @@ -1382,6 +1422,17 @@ shorten_branches (rtx first)
>           else
>             {
>               new_length = insn_current_length (insn);
> +             if (HAVE_ATTR_lock_length)
> +               {
> +                 int lock_length = insn_current_lock_length (insn);
> +
> +                 if (!increasing || lock_length > uid_lock_length[uid])
> +                   uid_lock_length[uid] = lock_length;
> +                 else
> +                   lock_length = uid_lock_length[uid];
> +                 if (new_length < lock_length)
> +                   new_length = lock_length;
> +               }
>               insn_current_address += new_length;
>             }
>
> @@ -1393,7 +1444,8 @@ shorten_branches (rtx first)
>  #endif
>
>           if (new_length != insn_lengths[uid]
> -             && (!increasing || new_length > insn_lengths[uid]))
> +             && (!increasing || HAVE_ATTR_lock_length
> +                 || new_length > insn_lengths[uid]))
>             {
>               insn_lengths[uid] = new_length;
>               something_changed = 1;
> @@ -1407,6 +1459,11 @@ shorten_branches (rtx first)
>      }
>
>    free (varying_length);
> +  if (HAVE_ATTR_lock_length)
> +    {
> +      free (uid_lock_length);
> +      uid_lock_length = 0;
> +    }
>  }
>
>  /* Given the body of an INSN known to be generated by an ASM statement,
> return
> Index: genattr.c
> ===================================================================
> --- genattr.c   (revision 2660)
> +++ genattr.c   (working copy)
> @@ -71,6 +71,10 @@ extern int insn_default_length (rtx);\n\
>  extern int insn_min_length (rtx);\n\
>  extern int insn_variable_length_p (rtx);\n\
>  extern int insn_current_length (rtx);\n\n\
> +extern int insn_default_lock_length (rtx);\n\
> +extern int insn_min_lock_length (rtx);\n\
> +extern int insn_variable_lock_length_p (rtx);\n\
> +extern int insn_current_lock_length (rtx);\n\n\
>  #include \"insn-addr.h\"\n");
>      }
>  }
> @@ -337,7 +341,8 @@ main (int argc, char **argv)
>      }
>
>    /* Special-purpose atributes should be tested with if, not #ifdef.  */
> -  const char * const special_attrs[] = { "length", "enabled", 0 };
> +  const char * const special_attrs[]
> +    = { "length", "lock_length", "enabled", 0 };
>    for (const char * const *p = special_attrs; *p; p++)
>      {
>        printf ("#ifndef HAVE_ATTR_%s\n"
> @@ -346,13 +351,19 @@ main (int argc, char **argv)
>      }
>    /* We make an exception here to provide stub definitions for
>       insn_*_length* functions.  */
> -  puts ("#if !HAVE_ATTR_length\n"
> -       "extern int hook_int_rtx_0 (rtx);\n"
> +  puts ("extern int hook_int_rtx_0 (rtx);\n"
> +       "#if !HAVE_ATTR_length\n"
>         "#define insn_default_length hook_int_rtx_0\n"
>         "#define insn_min_length hook_int_rtx_0\n"
>         "#define insn_variable_length_p hook_int_rtx_0\n"
>         "#define insn_current_length hook_int_rtx_0\n"
>         "#include \"insn-addr.h\"\n"
> +       "#endif\n"
> +       "#if !HAVE_ATTR_lock_length\n"
> +       "#define insn_default_lock_length hook_int_rtx_0\n"
> +       "#define insn_min_lock_length hook_int_rtx_0\n"
> +       "#define insn_variable_lock_length_p hook_int_rtx_0\n"
> +       "#define insn_current_lock_length hook_int_rtx_0\n"
>         "#endif\n");
>
>    /* Output flag masks for use by reorg.
> Index: genattrtab.c
> ===================================================================
> --- genattrtab.c        (revision 2660)
> +++ genattrtab.c        (working copy)
> @@ -242,6 +242,7 @@ struct attr_value_list **insn_code_value
>
>  static const char *alternative_name;
>  static const char *length_str;
> +static const char *lock_length_str;
>  static const char *delay_type_str;
>  static const char *delay_1_0_str;
>  static const char *num_delay_slots_str;
> @@ -1541,14 +1542,14 @@ substitute_address (rtx exp, rtx (*no_ad
>    */
>
>  static void
> -make_length_attrs (void)
> +make_length_attrs (const char **base)
>  {
>    static const char *new_names[] =
>      {
> -      "*insn_default_length",
> -      "*insn_min_length",
> -      "*insn_variable_length_p",
> -      "*insn_current_length"
> +      "*insn_default_%s",
> +      "*insn_min_%s",
> +      "*insn_variable_%s_p",
> +      "*insn_current_%s"
>      };
>    static rtx (*const no_address_fn[]) (rtx)
>      = {identity_fn,identity_fn, zero_fn, zero_fn};
> @@ -1561,7 +1562,7 @@ make_length_attrs (void)
>
>    /* See if length attribute is defined.  If so, it must be numeric.  Make
>       it special so we don't output anything for it.  */
> -  length_attr = find_attr (&length_str, 0);
> +  length_attr = find_attr (base, 0);
>    if (length_attr == 0)
>      return;
>
> @@ -1574,11 +1575,14 @@ make_length_attrs (void)
>    /* Make each new attribute, in turn.  */
>    for (i = 0; i < ARRAY_SIZE (new_names); i++)
>      {
> -      make_internal_attr (new_names[i],
> +      const char *p = attr_printf (strlen (new_names[i]) - 2 + strlen
> (*base),
> +                                  new_names[i], *base);
> +
> +      make_internal_attr (p,
>                           substitute_address
> (length_attr->default_val->value,
>                                               no_address_fn[i],
> address_fn[i]),
>                           ATTR_NONE);
> -      new_attr = find_attr (&new_names[i], 0);
> +      new_attr = find_attr (&p, 0);
>        for (av = length_attr->first_value; av; av = av->next)
>         for (ie = av->first_insn; ie; ie = ie->next)
>           {
> @@ -5179,6 +5183,7 @@ main (int argc, char **argv)
>
>    alternative_name = DEF_ATTR_STRING ("alternative");
>    length_str = DEF_ATTR_STRING ("length");
> +  lock_length_str = DEF_ATTR_STRING ("lock_length");
>    delay_type_str = DEF_ATTR_STRING ("*delay_type");
>    delay_1_0_str = DEF_ATTR_STRING ("*delay_1_0");
>    num_delay_slots_str = DEF_ATTR_STRING ("*num_delay_slots");
> @@ -5275,7 +5280,8 @@ main (int argc, char **argv)
>        fill_attr (attr);
>
>    /* Construct extra attributes for `length'.  */
> -  make_length_attrs ();
> +  make_length_attrs (&length_str);
> +  make_length_attrs (&lock_length_str);
>
>    /* Perform any possible optimizations to speed up compilation.  */
>    optimize_attrs ();
>
Joern Rennecke - Oct. 24, 2012, 1:33 p.m.
Quoting Richard Biener <richard.guenther@gmail.com>:

> Just to add some extra information, can you quote your ports
> ADJUST_INSN_LENGTH and one example instruction with length/lock_length
> attribute the above applies to?

This is a bit besides the point, since lock_length does mostly the traditional
branch shortening (with alignment effects rolled in for branches - I'm not
really happy about this, but that was required for convergence), length
does alignment effects on top, and ADJUST_INSN_LENGTH does random
context-sensitive adjustments related to pipeline quirks and conditional
execution.

I'm afraid this gcc port's logic in this area is both more complicated than
what you would expect in an average gcc port, and much more simplistic than
what a competent assembler programmer uses to optimize code for these
microarchitectures.

from arc.h:

#define ADJUST_INSN_LENGTH(X, LENGTH)                           \
   (LENGTH) += arc_adjust_insn_length ((X), (LENGTH))

from arc.c:

/* Return length adjustment for INSN.  */
int
arc_adjust_insn_length (rtx insn, int len)
{
   int adj = 0;

   if (!INSN_P (insn))
     return 0;
   if (GET_CODE (PATTERN (insn)) == SEQUENCE)
     {
       int adj0, adj1, len1;
       rtx pat = PATTERN (insn);
       rtx i0 = XVECEXP (pat, 0, 0);
       rtx i1 = XVECEXP (pat, 0, 1);

       len1 = get_attr_lock_length (i1);
       gcc_assert (!len || len >= 4 || (len == 2 && get_attr_iscompact (i1)));
       if (!len1)
         len1 = get_attr_iscompact (i1) != ISCOMPACT_FALSE ? 2 : 4;
       adj0 = arc_adjust_insn_length (i0, len - len1);
       adj1 = arc_adjust_insn_length (i1, len1);
       return adj0 + adj1;
     }
   if (recog_memoized (insn) == CODE_FOR_doloop_end_i)
     {
       rtx prev = prev_nonnote_insn (insn);

       return ((LABEL_P (prev)
                || (TARGET_ARC600
                    && (JUMP_P (prev) || GET_CODE (PATTERN (prev)) ==  
SEQUENCE)))
               ? 4 : 0);
     }

   /* Check for return with but one preceding insn since function
      start / call.  */
   if (TARGET_PAD_RETURN
       && JUMP_P (insn)
       && GET_CODE (PATTERN (insn)) != ADDR_VEC
       && GET_CODE (PATTERN (insn)) != ADDR_DIFF_VEC
       && get_attr_type (insn) == TYPE_RETURN)
     {
       rtx prev = prev_active_insn (insn);

       if (!prev || !(prev = prev_active_insn (prev))
           || ((NONJUMP_INSN_P (prev)
                && GET_CODE (PATTERN (prev)) == SEQUENCE)
               ? CALL_ATTR (XVECEXP (PATTERN (prev), 0, 0), NON_SIBCALL)
               : CALL_ATTR (prev, NON_SIBCALL)))
         return 4;
     }
   /* Rtl changes too much before arc_reorg to keep ccfsm state.
      But we are not required to give exact answers then.  */
   if (cfun->machine->arc_reorg_started
       && (JUMP_P (insn) || (len & 2)))
     {
       struct arc_ccfsm *statep = &cfun->machine->ccfsm_current;

       arc_ccfsm_advance_to (insn);
       switch (statep->state)
         {
         case 0:
           break;
         case 1: case 2:
           /* Deleted branch.  */
           return -len;
         case 3: case 4: case 5:
           /* Conditionalized insn.  */
           if ((!JUMP_P (insn)
                || (get_attr_type (insn) != TYPE_BRANCH
                    && get_attr_type (insn) != TYPE_UNCOND_BRANCH
                    && (get_attr_type (insn) != TYPE_RETURN
                        || (statep->cc != ARC_CC_EQ && statep->cc != ARC_CC_NE)
                        || NEXT_INSN (PREV_INSN (insn)) != insn)))
               && (len & 2))
             adj = 2;
           break;
         default:
           gcc_unreachable ();
         }
     }
   if (TARGET_ARC600)
     {
       rtx succ = next_real_insn (insn);

       if (succ && INSN_P (succ))
         adj += arc600_corereg_hazard (insn, succ);
     }

   /* Restore extracted operands - otherwise splitters like the  
addsi3_mixed one
      can go awry.  */
   extract_constrain_insn_cached (insn);

   return adj;
}

 From arc.md:

; Since the demise of REG_N_SETS, it is no longer possible to find out
; in the prologue / epilogue expanders how many times blink is set.
; Using df_regs_ever_live_p to decide if blink needs saving means that
; any explicit use of blink will cause it to be saved; hence we cannot
; represent the blink use in return / sibcall instructions themselves, and
; instead have to show it in EPILOGUE_USES and must explicitly
; forbid instructions that change blink in the return / sibcall delay slot.
(define_insn "return_i"
   [(return)]
   "reload_completed"
{
   rtx reg
     = gen_rtx_REG (Pmode,
                    arc_return_address_regs[arc_compute_function_type (cfun)]);

   if (TARGET_PAD_RETURN)
     arc_pad_return ();
   output_asm_insn (\"j%!%* [%0]%&\", &reg);
   return \"\";
}
   [(set_attr "type" "return")
    (set_attr "cond" "canuse")
    (set (attr "iscompact")
         (cond [(eq (symbol_ref "arc_compute_function_type (cfun)")
                    (symbol_ref "ARC_FUNCTION_NORMAL"))
                (const_string "maybe")]
               (const_string "false")))
    (set (attr "length")
         (cond [(eq (symbol_ref "arc_compute_function_type (cfun)")
                    (symbol_ref "ARC_FUNCTION_NORMAL"))
                (const_int 4)
                (and (match_test "0") (eq (match_dup 0) (pc)))
                (const_int 4)
                (eq_attr "verify_short" "no")
                (const_int 4)]
               (const_int 2)))])

Earlier in arc.md:

; When estimating sizes during arc_reorg, when optimizing for speed, there
; are three reasons why we need to consider branches to be length 6:
; - annull-false delay slot insns are implemented using conditional execution,
;   thus preventing short insn formation where used.
; - for ARC600: annull-true delay slot isnns are implemented where possile
;   using conditional execution, preventing short insn formation where used.
; - for ARC700: likely or somewhat likely taken branches are made long and
;   unaligned if possible to avoid branch penalty.
(define_insn "*branch_insn"
   [(set (pc)
         (if_then_else (match_operator 1 "proper_comparison_operator"
                                       [(reg CC_REG) (const_int 0)])
                       (label_ref (match_operand 0 "" ""))
                       (pc)))]
   ""
   "*
{
   if (arc_ccfsm_branch_deleted_p ())
     {
       arc_ccfsm_record_branch_deleted ();
       return \"; branch deleted, next insns conditionalized\";
     }
   else
     {
       arc_ccfsm_record_condition (operands[1], 0, insn, 0);
       if (get_attr_length (insn) == 2)
          return \"b%d1%? %^%l0%&\";
       else
          return \"b%d1%# %^%l0\";
     }
}"
   [(set_attr "type" "branch")
    (set
      (attr "lock_length")
      (cond [
        ; In arc_reorg we just guesstimate; might be more or less.
        (match_test "arc_branch_size_unknown_p ()")
        (const_int 4)

        (eq_attr "delay_slot_filled" "yes")
        (const_int 4)

        (ne
          (if_then_else
            (match_operand 1 "equality_comparison_operator" "")
            (ior (lt (minus (match_dup 0) (pc)) (const_int -512))
                 (gt (minus (match_dup 0) (pc))
                     (minus (const_int 506)
                            (symbol_ref "get_attr_delay_slot_length (insn)"))))
            (ior (match_test "!arc_short_comparison_p (operands[1], -64)")
                 (lt (minus (match_dup 0) (pc)) (const_int -64))
                 (gt (minus (match_dup 0) (pc))
                     (minus (const_int 58)
                            (symbol_ref "get_attr_delay_slot_length  
(insn)")))))
          (const_int 0))
        (const_int 4)

        (eq_attr "verify_short" "yes")
        (const_int 2)]
       (const_int 4)))
    (set (attr "iscompact")
         (cond [(eq_attr "lock_length" "2") (const_string "true")]
               (const_string "false")))])

> Which sounds straight-forward.  The docs of ADJUST_INSN_LENGTH
> are not entirely clear, but it sounds like it may only increase   
> length, correct?

No, it may decrease the length, but not below zero.

in fact, the arc port does decrease the length to zero in some cases
where a branch is deleted.

> I see that ADJUST_INSN_LENGTH is really not needed as everything
> should be expressable in the length attribute of an instruction?

In theory, yes, in a similar vein as you can use a turing machine to
implement a text editor.
Each different result length needs a different cond or if clause, so if
you have a C function that calculates the length, you need to evaluate
it multiple times.

>> If you define the @code{lock_length} attribute, the @code{lock_length}
>> attribute will be evaluated, and then the maximum with of @code{lock_length}
>
> with of?  I read it as 'of the'

I meant 'with the'.  I.e. maximum of current lock_length value and the  
(maximum)
lock_length value from the previous iteration.

>> value from the previous iteration will be formed and saved.
>
> So lock_length will only increase during iteration.

Yes.

>> Then the maximum of that value with the @code{length} attribute will
>> be formed, and @code{ADJUST_INSN_LENGTH} will be applied.
>
> ADJUST_INSN_LENGTH will be applied to the maximum?  What will
> be the 'instruction length' equivalent to the simple non-lock_length case?
> Is it the above, max (ADJUST_INSN_LENGTH (lock-length-max, length))?

Huh?  Your parentheses are muddled, so I'm not sure what exactly you
meant to ask.  And ADJUST_INSN_LENGTH is not really a function.  Well,
if we pretend that it was, the expression would be:

ADJUST_INSN_LENGTH (max (lock_length_max, length))

>> Thus, you can allow the length to vary downwards as well as upwards
>> across iterations with suitable definitions of the @code{length} attribute
>> and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
>> lead to infinite loops.
>
> I don't see that you can shrink length with just suitable lock_length and
> length attributes.

You can; have lock_length evaluate to something small, e.g. 0, and then
length dominates the value that is calculated each iteration.

>  What seems to be the cruical difference is that you
> apply ADJUST_INSN_LENGTH after combining lock-length-max and length.
> But then you _decrease_ length with ADJUST_INSN_LENGHT ...
>
> Maybe what you really want is ADJUST_INSN_LENGTH_AFTER which is
> applied afterwards?  Thus,

Maybe that would work, shifting the alignment computations into a
late-applied ADJUST_INSN_LENGTH; the ARC port has relations between  
length, lock_length, iscompact, and verify_short that form dependency  
circles.
When I try to unify length and lock_length,
genattrtab goes into an infinite loop, so there is no way of testing the
port till genattrtab knows when to quit.

> Note that
>
>> Care has to be taken that this does not
>> lead to infinite loops.
>
> doesn't convince me that is properly designed ;)

There are circles that emerge naturally from a description of the  
requirements.
Some local decisions need to be compromised to reach a globally stable
solution.  I don't see how the infrastructure can make a meaningful  
choice where this compromise should occur.
One thing that would be nice to have would be to have a tally of previous
values, so we could decide better when to compromise.

Well, there would be one way to offload the heavy lifting to the
infrastructure: have another attribute that assigns looping thresholds,
for the number of iterations where things change without a lock_length change.
The when an instruction breaks the limit, the new length, if larger  
than lock_length, would be stored in lock_length.
So in the simple length case, that threshold is zero.
What would you call such an attribute?
length_lock_threshold ?  length_iter_threshold ? length_iteration_limit ?
Joseph S. Myers - Oct. 24, 2012, 4:47 p.m.
On Tue, 23 Oct 2012, Joern Rennecke wrote:

> I'll be posting the ARC port shortly; it does not fit into a single 100 KB
> posting, so I'm thinking of splitting it in a configury patch and zx
> compressed files/tarballs for arc.c, arc.md, libgcc, and the rest of the port.

The size limit is 400 kB, not 100 kB.  Hopefully that means you don't need 
to compress so many bits and can attach more as plain patches for future 
revisions.

Patch

Index: doc/md.texi
===================================================================
--- doc/md.texi	(revision 2660)
+++ doc/md.texi	(working copy)
@@ -7515,6 +7515,9 @@  extern enum attr_type get_attr_type ();
 code chunks.  This is especially important when verifying branch
 distances. @xref{Insn Lengths}.
 
+@item lock_length
+The @code{lock_length} attribute.
+
 @item enabled
 The @code{enabled} attribute can be defined to prevent certain
 alternatives of an insn definition from being used during code
@@ -8038,6 +8041,22 @@  (define_insn "jump"
                       (const_int 6)))])
 @end smallexample
 
+@cindex lock_length
+Usually, when doing optimizing branch shortening, the instruction length
+is calculated by avaluating the @code{length} attribute, applying
+@code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
+value and the length of the instruction in the previous iteration.
+
+If you define the @code{lock_length} attribute, the @code{lock_length}
+attribute will be evaluated, and then the maximum with of @code{lock_length}
+value from the previous iteration will be formed and saved.
+Then the maximum of that value with the @code{length} attribute will
+be formed, and @code{ADJUST_INSN_LENGTH} will be applied.
+Thus, you can allow the length to vary downwards as well as upwards
+across iterations with suitable definitions of the @code{length} attribute
+and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
+lead to infinite loops.
+
 @end ifset
 @ifset INTERNALS
 @node Constant Attributes
Index: final.c
===================================================================
--- final.c	(revision 2660)
+++ final.c	(working copy)
@@ -308,6 +308,7 @@  dbr_sequence_length (void)
    `insn_current_length'.  */
 
 static int *insn_lengths;
+static int *uid_lock_length;
 
 VEC(int,heap) *insn_addresses_;
 
@@ -430,14 +431,41 @@  get_attr_length_1 (rtx insn, int (*fallb
   return length;
 }
 
+/* Calculate the maximum length of INSN.  */
+static int
+insn_max_length (rtx insn)
+{
+  int length, lock_length;
+
+  length = insn_default_length (insn);
+  if (HAVE_ATTR_lock_length)
+    {
+      lock_length = insn_default_lock_length (insn);
+      if (length < lock_length)
+	length = lock_length;
+    }
+  return length;
+}
+
 /* Obtain the current length of an insn.  If branch shortening has been done,
    get its actual length.  Otherwise, get its maximum length.  */
 int
 get_attr_length (rtx insn)
 {
-  return get_attr_length_1 (insn, insn_default_length);
+  return get_attr_length_1 (insn, insn_max_length);
 }
 
+int
+get_attr_lock_length (rtx insn)
+{
+  if (uid_lock_length && insn_lengths_max_uid > INSN_UID (insn))
+    return uid_lock_length[INSN_UID (insn)];
+  return get_attr_length_1 (insn, insn_min_lock_length);
+}
+
+#define INSN_VARIABLE_LENGTH_P(INSN) \
+  (insn_variable_length_p (INSN) || insn_variable_lock_length_p (INSN))
+
 /* Obtain the current length of an insn.  If branch shortening has been done,
    get its actual length.  Otherwise, get its minimum length.  */
 int
@@ -966,6 +994,11 @@  shorten_branches (rtx first)
   /* Allocate the rest of the arrays.  */
   insn_lengths = XNEWVEC (int, max_uid);
   insn_lengths_max_uid = max_uid;
+  if (HAVE_ATTR_lock_length)
+    uid_lock_length = XCNEWVEC (int, max_uid);
+  else
+    uid_lock_length = insn_lengths;
+
   /* Syntax errors can lead to labels being outside of the main insn stream.
      Initialize insn_addresses, so that we get reproducible results.  */
   INSN_ADDRESSES_ALLOC (max_uid);
@@ -1064,7 +1097,7 @@  shorten_branches (rtx first)
 #endif /* CASE_VECTOR_SHORTEN_MODE */
 
   /* Compute initial lengths, addresses, and varying flags for each insn.  */
-  int (*length_fun) (rtx) = increasing ? insn_min_length : insn_default_length;
+  int (*length_fun) (rtx) = increasing ? insn_min_length : insn_max_length;
 
   for (insn_current_address = 0, insn = first;
        insn != 0;
@@ -1106,7 +1139,7 @@  shorten_branches (rtx first)
 	  /* Alignment is handled by ADDR_VEC_ALIGN.  */
 	}
       else if (GET_CODE (body) == ASM_INPUT || asm_noperands (body) >= 0)
-	insn_lengths[uid] = asm_insn_count (body) * insn_default_length (insn);
+	insn_lengths[uid] = asm_insn_count (body) * insn_max_length (insn);
       else if (GET_CODE (body) == SEQUENCE)
 	{
 	  int i;
@@ -1117,7 +1150,7 @@  shorten_branches (rtx first)
 	  const_delay_slots = 0;
 #endif
 	  int (*inner_length_fun) (rtx)
-	    = const_delay_slots ? length_fun : insn_default_length;
+	    = const_delay_slots ? length_fun : insn_max_length;
 	  /* Inside a delay slot sequence, we do not do any branch shortening
 	     if the shortening could change the number of delay slots
 	     of the branch.  */
@@ -1130,7 +1163,7 @@  shorten_branches (rtx first)
 	      if (GET_CODE (body) == ASM_INPUT
 		  || asm_noperands (PATTERN (XVECEXP (body, 0, i))) >= 0)
 		inner_length = (asm_insn_count (PATTERN (inner_insn))
-				* insn_default_length (inner_insn));
+				* insn_max_length (inner_insn));
 	      else
 		inner_length = inner_length_fun (inner_insn);
 
@@ -1138,7 +1171,7 @@  shorten_branches (rtx first)
 	      if (const_delay_slots)
 		{
 		  if ((varying_length[inner_uid]
-		       = insn_variable_length_p (inner_insn)) != 0)
+		       = INSN_VARIABLE_LENGTH_P (inner_insn)) != 0)
 		    varying_length[uid] = 1;
 		  INSN_ADDRESSES (inner_uid) = (insn_current_address
 						+ insn_lengths[uid]);
@@ -1151,7 +1184,7 @@  shorten_branches (rtx first)
       else if (GET_CODE (body) != USE && GET_CODE (body) != CLOBBER)
 	{
 	  insn_lengths[uid] = length_fun (insn);
-	  varying_length[uid] = insn_variable_length_p (insn);
+	  varying_length[uid] = INSN_VARIABLE_LENGTH_P (insn);
 	}
 
       /* If needed, do any adjustment.  */
@@ -1354,7 +1387,7 @@  shorten_branches (rtx first)
 		{
 		  rtx inner_insn = XVECEXP (body, 0, i);
 		  int inner_uid = INSN_UID (inner_insn);
-		  int inner_length;
+		  int inner_length, lock_length;
 
 		  INSN_ADDRESSES (inner_uid) = insn_current_address;
 
@@ -1365,16 +1398,23 @@  shorten_branches (rtx first)
 		  else
 		    inner_length = insn_current_length (inner_insn);
 
-		  if (inner_length != insn_lengths[inner_uid])
+		  lock_length
+		    = (HAVE_ATTR_lock_length
+		       ? insn_current_lock_length (inner_insn) : inner_length);
+		  if (lock_length != uid_lock_length[inner_uid])
 		    {
-		      if (!increasing || inner_length > insn_lengths[inner_uid])
+		      if (!increasing
+			  || lock_length > uid_lock_length[inner_uid])
 			{
-			  insn_lengths[inner_uid] = inner_length;
+			  uid_lock_length[inner_uid] = lock_length;
 			  something_changed = 1;
 			}
 		      else
-			inner_length = insn_lengths[inner_uid];
+			lock_length = uid_lock_length[inner_uid];
 		    }
+		  if (inner_length < lock_length)
+		    inner_length = lock_length;
+		  insn_lengths[inner_uid] = inner_length;
 		  insn_current_address += inner_length;
 		  new_length += inner_length;
 		}
@@ -1382,6 +1422,17 @@  shorten_branches (rtx first)
 	  else
 	    {
 	      new_length = insn_current_length (insn);
+	      if (HAVE_ATTR_lock_length)
+		{
+		  int lock_length = insn_current_lock_length (insn);
+
+		  if (!increasing || lock_length > uid_lock_length[uid])
+		    uid_lock_length[uid] = lock_length;
+		  else
+		    lock_length = uid_lock_length[uid];
+		  if (new_length < lock_length)
+		    new_length = lock_length;
+		}
 	      insn_current_address += new_length;
 	    }
 
@@ -1393,7 +1444,8 @@  shorten_branches (rtx first)
 #endif
 
 	  if (new_length != insn_lengths[uid]
-	      && (!increasing || new_length > insn_lengths[uid]))
+	      && (!increasing || HAVE_ATTR_lock_length
+		  || new_length > insn_lengths[uid]))
 	    {
 	      insn_lengths[uid] = new_length;
 	      something_changed = 1;
@@ -1407,6 +1459,11 @@  shorten_branches (rtx first)
     }
 
   free (varying_length);
+  if (HAVE_ATTR_lock_length)
+    {
+      free (uid_lock_length);
+      uid_lock_length = 0;
+    }
 }
 
 /* Given the body of an INSN known to be generated by an ASM statement, return
Index: genattr.c
===================================================================
--- genattr.c	(revision 2660)
+++ genattr.c	(working copy)
@@ -71,6 +71,10 @@  extern int insn_default_length (rtx);\n\
 extern int insn_min_length (rtx);\n\
 extern int insn_variable_length_p (rtx);\n\
 extern int insn_current_length (rtx);\n\n\
+extern int insn_default_lock_length (rtx);\n\
+extern int insn_min_lock_length (rtx);\n\
+extern int insn_variable_lock_length_p (rtx);\n\
+extern int insn_current_lock_length (rtx);\n\n\
 #include \"insn-addr.h\"\n");
     }
 }
@@ -337,7 +341,8 @@  main (int argc, char **argv)
     }
 
   /* Special-purpose atributes should be tested with if, not #ifdef.  */
-  const char * const special_attrs[] = { "length", "enabled", 0 };
+  const char * const special_attrs[]
+    = { "length", "lock_length", "enabled", 0 };
   for (const char * const *p = special_attrs; *p; p++)
     {
       printf ("#ifndef HAVE_ATTR_%s\n"
@@ -346,13 +351,19 @@  main (int argc, char **argv)
     }
   /* We make an exception here to provide stub definitions for
      insn_*_length* functions.  */
-  puts ("#if !HAVE_ATTR_length\n"
-	"extern int hook_int_rtx_0 (rtx);\n"
+  puts ("extern int hook_int_rtx_0 (rtx);\n"
+	"#if !HAVE_ATTR_length\n"
 	"#define insn_default_length hook_int_rtx_0\n"
 	"#define insn_min_length hook_int_rtx_0\n"
 	"#define insn_variable_length_p hook_int_rtx_0\n"
 	"#define insn_current_length hook_int_rtx_0\n"
 	"#include \"insn-addr.h\"\n"
+	"#endif\n"
+	"#if !HAVE_ATTR_lock_length\n"
+	"#define insn_default_lock_length hook_int_rtx_0\n"
+	"#define insn_min_lock_length hook_int_rtx_0\n"
+	"#define insn_variable_lock_length_p hook_int_rtx_0\n"
+	"#define insn_current_lock_length hook_int_rtx_0\n"
 	"#endif\n");
 
   /* Output flag masks for use by reorg.
Index: genattrtab.c
===================================================================
--- genattrtab.c	(revision 2660)
+++ genattrtab.c	(working copy)
@@ -242,6 +242,7 @@  struct attr_value_list **insn_code_value
 
 static const char *alternative_name;
 static const char *length_str;
+static const char *lock_length_str;
 static const char *delay_type_str;
 static const char *delay_1_0_str;
 static const char *num_delay_slots_str;
@@ -1541,14 +1542,14 @@  substitute_address (rtx exp, rtx (*no_ad
   */
 
 static void
-make_length_attrs (void)
+make_length_attrs (const char **base)
 {
   static const char *new_names[] =
     {
-      "*insn_default_length",
-      "*insn_min_length",
-      "*insn_variable_length_p",
-      "*insn_current_length"
+      "*insn_default_%s",
+      "*insn_min_%s",
+      "*insn_variable_%s_p",
+      "*insn_current_%s"
     };
   static rtx (*const no_address_fn[]) (rtx)
     = {identity_fn,identity_fn, zero_fn, zero_fn};
@@ -1561,7 +1562,7 @@  make_length_attrs (void)
 
   /* See if length attribute is defined.  If so, it must be numeric.  Make
      it special so we don't output anything for it.  */
-  length_attr = find_attr (&length_str, 0);
+  length_attr = find_attr (base, 0);
   if (length_attr == 0)
     return;
 
@@ -1574,11 +1575,14 @@  make_length_attrs (void)
   /* Make each new attribute, in turn.  */
   for (i = 0; i < ARRAY_SIZE (new_names); i++)
     {
-      make_internal_attr (new_names[i],
+      const char *p = attr_printf (strlen (new_names[i]) - 2 + strlen (*base),
+				   new_names[i], *base);
+
+      make_internal_attr (p,
 			  substitute_address (length_attr->default_val->value,
 					      no_address_fn[i], address_fn[i]),
 			  ATTR_NONE);
-      new_attr = find_attr (&new_names[i], 0);
+      new_attr = find_attr (&p, 0);
       for (av = length_attr->first_value; av; av = av->next)
 	for (ie = av->first_insn; ie; ie = ie->next)
 	  {
@@ -5179,6 +5183,7 @@  main (int argc, char **argv)
 
   alternative_name = DEF_ATTR_STRING ("alternative");
   length_str = DEF_ATTR_STRING ("length");
+  lock_length_str = DEF_ATTR_STRING ("lock_length");
   delay_type_str = DEF_ATTR_STRING ("*delay_type");
   delay_1_0_str = DEF_ATTR_STRING ("*delay_1_0");
   num_delay_slots_str = DEF_ATTR_STRING ("*num_delay_slots");
@@ -5275,7 +5280,8 @@  main (int argc, char **argv)
       fill_attr (attr);
 
   /* Construct extra attributes for `length'.  */
-  make_length_attrs ();
+  make_length_attrs (&length_str);
+  make_length_attrs (&lock_length_str);
 
   /* Perform any possible optimizations to speed up compilation.  */
   optimize_attrs ();