Ping: RFA: add lock_length attribute to break branch-shortening cycles

[Richard Sandiford]

Joern Rennecke <joern.rennecke@embecosm.com> writes:
> @@ -1360,12 +1369,20 @@ shorten_branches (rtx first ATTRIBUTE_UN
>  		  else
>  		    inner_length = insn_current_length (inner_insn);
>  
> -		  if (inner_length != insn_lengths[inner_uid])
> +		  /* We can't record lengths of delay slot insns, because
> +		     they might just be a copy of the insn at the branch
> +		     target, sharing its uid.  */
> +		  if (i == 0 && inner_length != insn_lengths[inner_uid])
>  		    {
> -		      insn_lengths[inner_uid] = inner_length;
> -		      something_changed = 1;
> +		      if (inner_length > insn_lengths[inner_uid] || first_pass)
> +			{
> +			  insn_lengths[inner_uid] = inner_length;
> +			  something_changed = 1;
> +			}
> +		      else
> +			inner_length = insn_lengths[inner_uid];
>  		    }
> -		  insn_current_address += insn_lengths[inner_uid];
> +		  insn_current_address += inner_length;
>  		  new_length += inner_length;
>  		}
>  	    }

The fact that we even have shared unique ids is pretty bad -- and surely
a contradiction in terms -- but I think both ways of handling them rely
on the length being the same for all copies.  If we don't record a length
(your version), we won't set something_changed if the length of this copy
did in fact change, which could lead to branches remaining too short.
If we do record a length (current version), we could end up changing
the length of previous copies that have already been processed in
this iteration.  Both would be wrong, but at least the current version
is simpler and means we get the natural behaviour if anyone ever does
"fix" dbr_schedule.

I think first_pass should be !optimize.  The set-up loop chooses minimum
lengths when optimising, so it should be correct to keep the maximum on
all iterations of this loop.  I.e.

		  if (optimize
		      ? inner_length > insn_lengths[inner_uid]
		      : inner_length < insn_lengths[inner_uid])
		    {
		      insn_lengths[inner_uid] = inner_length;
		      something_changed = 1;
		    }
		  else
		    inner_length = insn_lengths[inner_uid];

> @@ -1382,7 +1399,8 @@ shorten_branches (rtx first ATTRIBUTE_UN
>  	  insn_current_address += (new_length - tmp_length);
>  #endif
>  
> -	  if (new_length != insn_lengths[uid])
> +	  if (new_length != insn_lengths[uid]
> +	      && (new_length > insn_lengths[uid] || first_pass))
>  	    {
>  	      insn_lengths[uid] = new_length;
>  	      something_changed = 1;

Same first_pass comment here.  I.e.:

	  if (optimize
	      ? new_length > insn_lengths[uid]
	      : new_length < insn_lengths[uid])

I think you need an "else" that does:

    insn_current_address += insn_lengths[uid] - new_length;

Richard