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

[Joern Rennecke <joern dot rennecke at embecosm dot com>]

Quoting Richard Sandiford <rdsandiford@googlemail.com>:

> I think instead the set-up loop should have:
>
>       if (GET_CODE (body) == ADDR_VEC || GET_CODE (body) == ADDR_DIFF_VEC)
> 	{
> #ifdef CASE_VECTOR_SHORTEN_MODE
> 	  if (increasing && GET_CODE (body) == ADDR_DIFF_VEC)
> 	    PUT_MODE (body, CASE_VECTOR_SHORTEN_MODE (0, 0, body));
> #endif
> 	  /* This only takes room if read-only data goes into the text
> 	     section.  */
> 	  if (JUMP_TABLES_IN_TEXT_SECTION
> 	      || readonly_data_section == text_section)
> 	    insn_lengths[uid] = (XVECLEN (body,
> 					  GET_CODE (body) == ADDR_DIFF_VEC)
> 				 * GET_MODE_SIZE (GET_MODE (body)));
> 	  /* Alignment is handled by ADDR_VEC_ALIGN.  */
> 	}
>
> (with just the CASE_VECTOR_SHORTEN_MODE part being new).
> We then start with the most optimistic length possible,
> as with everything else.

Well, ports could always tailor the initial mode with CASE_VECTOR_MODE,
but it is indeed simpler when the branch shortening pass provide a
sensible initialization.

How about putting this at the end of this block:
#ifdef CASE_VECTOR_SHORTEN_MODE
  if (optimize)
    {
      /* Look for ADDR_DIFF_VECs, and initialize their minimum and maximum
         label fields.  */

Of course increasing would have to be set earlier.

With regards to in what way it would make sense to change increasing to
something other than optimize, I think we could have a flag to control
this, which is turned on by default at -O1; it could then be turned off
if people want only some other (quicker?) optimizations, or if they want
to work around a machine-specific bug triggered by a single source file.
I.e. optimize might be set when increasing is not.  Vice versa it makes
little sense - iterating branch shortening is certainly an optimization.
So is using CASE_VECTOR_SHORTEN_MODE, but it does not require iterating,
as we've already calculated the required addresses in the preliminary pass
before the main loop.
So it makes sense to keep the condition for CASE_VECTOR_SHORTEN_MODE
as 'optimize' (or use a different flag, e.g. case_vector_shorten_p),
and have this at the end of this initial CASE_VECTOR_SHORTEN_MODE block:

          flags.min_after_base = min > rel;
          flags.max_after_base = max > rel;
          ADDR_DIFF_VEC_FLAGS (pat) = flags;

+         if (increasing)
+           PUT_MODE (body, CASE_VECTOR_SHORTEN_MODE (0, 0, body));
        }
    }
#endif /* CASE_VECTOR_SHORTEN_MODE */

              continue;
            }
#endif /* CASE_VECTOR_SHORTEN_MODE */



> The main shortening if statement should then be conditional on:
>
> #ifdef CASE_VECTOR_SHORTEN_MODE
> 	  if (increasing
> 	      && JUMP_P (insn)
> 	      && GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC)

As explained above, CASE_VECTOR_SHORTEN_MODE can make in the
decreasing/non-iterating branch shortening mode.

so I think the code for changing the mode should be
               if (!increasing
                  || (GET_MODE_SIZE (vec_mode)
                      >= GET_MODE_SIZE (GET_MODE (body))))
                PUT_MODE (body, vec_mode);