[PATCH] PPC: drop redundant value conversion from md_assemble()

Jan Beulich jbeulich@suse.com
Tue Dec 17 08:24:45 GMT 2024


On 17.12.2024 00:42, Alan Modra wrote:
> On Mon, Dec 16, 2024 at 12:40:22PM +0100, Jan Beulich wrote:
>> Just ahead of the enclosing OBJ_ELF conditional the exact same
>> conversion was already carried out, with "val" not further changed in
>> between.
> 
> Thanks, yes, that duplication should be removed.
> 
>> Doesn't X_unsigned also need taking into account in the remaining
>> identical piece of code?
> 
> Maybe.  In adding that piece of code, I was just interested in a very
> limited set of expressions.  The trouble with caring too much about
> correctness is that someone will come along and tell you that despite
> your best efforts you still didn't get it correct.  ;-)

Right, I'm fully aware of this with past and present efforts to make
expression evaluation more predictable for "uncommon" cases. Let me
widen the question and pull in Nick as well.

According to a comment in expr.h, X_extrabit was introduced "so that
e.g. expressions used with .sleb128 directives can use the full range
available for an unsigned word, but can also properly represent all
values of a signed word." That alone, however, can be achieved with
properly setting X_unsigned, afaict. I'm therefore wondering whether
the comment is inaccurate (potentially [also] meaning "the full range
available for a negated unsigned word"), or whether X_extrabit should
actually be dropped again, ensuring X_unsigned is properly set in all
relevant places (in particular everywhere operations on two
O_constant-s are resolved).

Resolving the above of course is also somewhat related to the fact
that we resolve operations on constants in three places (expr(),
resolve_expression(), and resolve_symbol_value()), in three slightly
(or not so slightly) different ways. For this I haven't come to a
reasonable conclusion yet; one way or another we surely would benefit
from doing all of this in just one place.

And then finally (for the moment; surely there are more issues
lurking), there's an issue with .octa. By analogy, .octa on a BFD64
build ought to function similar to .quad on a !BFD64 build. The
latter, while (naturally) limited in value range, can still be used
with expressions which can only be resolved at the end of assembly.
.octa, otoh, can't be, as there's no way to internally represent the
necessary relocation. I'm uncertain, though, about the seemingly
"natural" approach of simply adding BFD_RELOC_128 and BFD_RELOC_PC128.
That's mainly connected to me not being certain whether I actually see
all the potential implications particularly on target-specific code
when adding such generic (internal) relocation types.

Jan


More information about the Binutils mailing list