[PATCH v4 6/7] Support different fragment ordering in frag_offset_ignore_align_p

[Jan Beulich]

On 15.08.2025 00:50, Evgeny Karpov wrote:
> This patch extends frag_offset_ignore_align_p to support different fragment
> order in the same way it is done in frag_offset_fixed_p.

What is missing here is discussion of the correctness of the change wrt
existing callers (afaics there's exactly one).

> gas/ChangeLog:
> 
> 	* frags.c (frag_offset_ignore_align_p): Update.

Seeing that patch 5 uses the function, don't the two patches need to go the
other way around? Or else why would the change be needed?

> --- a/gas/frags.c
> +++ b/gas/frags.c
> @@ -456,8 +456,8 @@ frag_offset_fixed_p (const fragS *frag1, const
> fragS *frag2, offsetT *offset)
>    return false;
>  }
> 
> -/* Return TRUE if FRAG2 follows FRAG1 with a fixed relationship
> -   between the two assuming alignment frags do nothing.  Set OFFSET to
> +/* Return TRUE if FRAG1 and FRAG2 have a fixed relationship between
> +   their start addresses assuming alignment frags do nothing.  Set OFFSET to
>     the difference in address not already accounted for in the frag
>     FR_ADDRESS.  */

With or without your change, imo the first sentence would read quite a bit
easier with a comma ahead of "assuming". Please can you take the opportunity
and add one?

> @@ -495,6 +495,25 @@ frag_offset_ignore_align_p (const fragS *frag1,
> const fragS *frag2,
>  	}
>      }
> 
> +  /* Maybe frag1 is after frag2.  */

A similar comment wants adding ahead of the earlier block.

> +  off = frag1->fr_address - frag2->fr_address;
> +  frag = frag2;
> +  while (frag->fr_type == rs_fill
> +	 || frag->fr_type == rs_align
> +	 || frag->fr_type == rs_align_code
> +	 || frag->fr_type == rs_align_test)
> +    {
> +      off -= frag->fr_fix + frag->fr_offset * frag->fr_var;

This line isn't unconditional in the earlier block. Any difference will
want extra explanation in the description.

> +      frag = frag->fr_next;
> +      if (frag == NULL)
> +	break;
> +      if (frag == frag1)
> +	{
> +	  *offset = off;
> +	  return true;
> +	}
> +    }
> +
>    return false;
>  }
>