[1/5] gas: streamline expr_build_dot()

  There's no point involving symbol_clone_if_forward_ref(), just for it to
replace dot_symbol by one obtained from symbol_temp_new_now(). For the
abs-section case also produce a slightly more "complete" (as in: all
potentially relevant fields filled) expression by going through
expr_build_uconstant().

Move the function next to current_location(), for it to be easier to see
the (dis)similarities. Correct the function's comment while there.
  

On Fri, 22 Nov 2024, Jan Beulich wrote:

> There's no point involving symbol_clone_if_forward_ref(), just for it to
> replace dot_symbol by one obtained from symbol_temp_new_now(). For the
> abs-section case also produce a slightly more "complete" (as in: all
> potentially relevant fields filled) expression by going through
> expr_build_uconstant().

 As I recall this cloning is needed for equated symbols, so that the value 
of `.' used in an expression used for a symbol's calculation is obtained 
at the time of the symbol's reference and not the symbol's definition.

 For further details please see the discussion starting from: 
<https://inbox.sourceware.org/binutils/alpine.DEB.1.10.1010291442440.25860@tp.orcam.me.uk/>.

 Let me know if you have further questions after reviewing it and I'll try 
to address them (mind though that it's been 14 years since I worked on it 
and my memory may a bit hazy about it now).

  Maciej
  

On 26.11.2024 03:43, Maciej W. Rozycki wrote:
> On Fri, 22 Nov 2024, Jan Beulich wrote:
> 
>> There's no point involving symbol_clone_if_forward_ref(), just for it to
>> replace dot_symbol by one obtained from symbol_temp_new_now(). For the
>> abs-section case also produce a slightly more "complete" (as in: all
>> potentially relevant fields filled) expression by going through
>> expr_build_uconstant().
> 
>  As I recall this cloning is needed for equated symbols, so that the value 
> of `.' used in an expression used for a symbol's calculation is obtained 
> at the time of the symbol's reference and not the symbol's definition.

Yet then symbol_clone_if_forward_ref() would be wrong to use here, as that
pins down . at the present location (the cloning is done to keep this
"pinning down" from affecting the original symbol). Aiui it's quite the
opposite: expr_build_dot() _wants_ to latch . at its current location. At
least according to my interpretation of all the uses of it that we
presently have.

The special treatment of forward equates is happening elsewhere, by
pseudo_set() using deferred_expression() in that case. That's entirely
unrelated to expr_build_dot() (current_location() is what the expression
parser uses).

Jan
  

On Tue, 26 Nov 2024, Jan Beulich wrote:

> >> There's no point involving symbol_clone_if_forward_ref(), just for it to
> >> replace dot_symbol by one obtained from symbol_temp_new_now(). For the
> >> abs-section case also produce a slightly more "complete" (as in: all
> >> potentially relevant fields filled) expression by going through
> >> expr_build_uconstant().
> > 
> >  As I recall this cloning is needed for equated symbols, so that the value 
> > of `.' used in an expression used for a symbol's calculation is obtained 
> > at the time of the symbol's reference and not the symbol's definition.
> 
> Yet then symbol_clone_if_forward_ref() would be wrong to use here, as that
> pins down . at the present location (the cloning is done to keep this
> "pinning down" from affecting the original symbol). Aiui it's quite the
> opposite: expr_build_dot() _wants_ to latch . at its current location. At
> least according to my interpretation of all the uses of it that we
> presently have.

 Correct indeed, I misremembered.

 I'm not convinced by your simplification though, as the key here is to
tie `expr_build_dot' to `dot_symbol', even if it adds extra layering and 
effectively boils down to the same calculation your proposed replacement 
does.  This layering is so that we have a single tracking symbol, as 
proposed by Richard Sandiford to represent ".".  See the discussion in: 
<https://inbox.sourceware.org/binutils/alpine.DEB.1.10.1010291442440.25860@tp.orcam.me.uk/> 
for the rationale.

 Any inconsistency between `dot_symbol' and `dot_frag'/`dot_value' can I 
think be sorted by setting the latter values to corresponding pieces of 
information retrieved from `dot_symbol', or maybe even discarding them as 
redundant.

 I can see there are three places where they are set, namely `md_assemble' 
in the POWER backend (which I suppose knows what it's doing, so should 
presumably just call `symbol_set_value_now (&dot_symbol)' instead), and 
then `expr' and `emit_expr_with_reloc'.  Perhaps the latter two can be 
discarded now that we have `dot_symbol' updated at every assembly line, 
but that would have to be verified.  I don't know offhand.

  Maciej
  

On Wed, 4 Dec 2024, Maciej W. Rozycki wrote:

>  I'm not convinced by your simplification though, as the key here is to
> tie `expr_build_dot' to `dot_symbol', even if it adds extra layering and 
> effectively boils down to the same calculation your proposed replacement 
> does.  This layering is so that we have a single tracking symbol, as 
> proposed by Richard Sandiford to represent ".".  See the discussion in: 
> <https://inbox.sourceware.org/binutils/alpine.DEB.1.10.1010291442440.25860@tp.orcam.me.uk/> 
> for the rationale.

 I didn't notice you actually already committed the change.  Oh well.  It 
took me a couple of hours to refresh myself on this subject in the hectic 
time I've been through recently.  I guess I didn't make it, but then I did 
not consider this discussion closed yet just by the passing of time.

 As I say I'm not convinced it's an improvement even if the resulting code 
looks simpler, because it my view it breaks the structure around the dot 
symbol.  I think the change should be reverted unless you have really 
strong arguments in favour to keeping it.

  Maciej
  

On 04.12.2024 02:46, Maciej W. Rozycki wrote:
> On Tue, 26 Nov 2024, Jan Beulich wrote:
> 
>>>> There's no point involving symbol_clone_if_forward_ref(), just for it to
>>>> replace dot_symbol by one obtained from symbol_temp_new_now(). For the
>>>> abs-section case also produce a slightly more "complete" (as in: all
>>>> potentially relevant fields filled) expression by going through
>>>> expr_build_uconstant().
>>>
>>>  As I recall this cloning is needed for equated symbols, so that the value 
>>> of `.' used in an expression used for a symbol's calculation is obtained 
>>> at the time of the symbol's reference and not the symbol's definition.
>>
>> Yet then symbol_clone_if_forward_ref() would be wrong to use here, as that
>> pins down . at the present location (the cloning is done to keep this
>> "pinning down" from affecting the original symbol). Aiui it's quite the
>> opposite: expr_build_dot() _wants_ to latch . at its current location. At
>> least according to my interpretation of all the uses of it that we
>> presently have.
> 
>  Correct indeed, I misremembered.
> 
>  I'm not convinced by your simplification though, as the key here is to
> tie `expr_build_dot' to `dot_symbol', even if it adds extra layering and 
> effectively boils down to the same calculation your proposed replacement 
> does.  This layering is so that we have a single tracking symbol, as 
> proposed by Richard Sandiford to represent ".".  See the discussion in: 
> <https://inbox.sourceware.org/binutils/alpine.DEB.1.10.1010291442440.25860@tp.orcam.me.uk/> 
> for the rationale.
> 
>  Any inconsistency between `dot_symbol' and `dot_frag'/`dot_value' can I 
> think be sorted by setting the latter values to corresponding pieces of 
> information retrieved from `dot_symbol', or maybe even discarding them as 
> redundant.
> 
>  I can see there are three places where they are set, namely `md_assemble' 
> in the POWER backend (which I suppose knows what it's doing, so should 
> presumably just call `symbol_set_value_now (&dot_symbol)' instead), and 
> then `expr' and `emit_expr_with_reloc'.  Perhaps the latter two can be 
> discarded now that we have `dot_symbol' updated at every assembly line, 
> but that would have to be verified.  I don't know offhand.

I don't think they can be deleted, or else e.g.

d0:
	.long .-d0, .-d0, .-d0

would emit three times the same value, I expect.

Jan