Re: [PATCH] Properly honor param_max_fsm_thread_path_insns in backwards threader

[Jeff Law <jeffreyalaw@gmail.com>]

On 8/2/2022 7:00 AM, Richard Biener via Gcc-patches wrote:
> I am trying to make sense of back_threader_profitability::profitable_path_p
> and the first thing I notice is that we do
>
>    /* Threading is profitable if the path duplicated is hot but also
>       in a case we separate cold path from hot path and permit optimization
>       of the hot path later.  Be on the agressive side here. In some testcases,
>       as in PR 78407 this leads to noticeable improvements.  */
>    if (m_speed_p
>        && ((taken_edge && optimize_edge_for_speed_p (taken_edge))
>            || contains_hot_bb))
>      {
>        if (n_insns >= param_max_fsm_thread_path_insns)
>          {
>            if (dump_file && (dump_flags & TDF_DETAILS))
>              fprintf (dump_file, "  FAIL: Jump-thread path not considered: "
>                       "the number of instructions on the path "
>                       "exceeds PARAM_MAX_FSM_THREAD_PATH_INSNS.\n");
>            return false;
>          }
> ...
>      }
>    else if (!m_speed_p && n_insns > 1)
>      {
>        if (dump_file && (dump_flags & TDF_DETAILS))
>          fprintf (dump_file, "  FAIL: Jump-thread path not considered: "
>                   "duplication of %i insns is needed and optimizing for size.\n",
>                   n_insns);
>        return false;
>      }
> ...
>    return true;
>
> thus we apply the n_insns >= param_max_fsm_thread_path_insns only
> to "hot paths".  The comment above this isn't entirely clear whether
> this is by design ("Be on the aggressive side here ...") but I think
> this is a mistake.  In fact the "hot path" check seems entirely
> useless since if the path is not hot we simply continue threading it.
I think the intent here was to allow more insns to be copied if the path 
was hot than if it was not not hot.     But the logic seems a bit 
convoluted here.

jeff