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

[Nick Clifton]

Hi Jan, Hi Alan,

>>> 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.  ;-)

> 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).

I get that doing this would help to simplify things and so make the code
cleaner.  But would it really be worth it, given the potential for
introducing new bugs ?  Ie, if it ain't broke, don't fix it.


> 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.

Agreed - in principle at least.  All three functions are quite complex
however, so merging them would be a difficult process.  Perhaps if you
are able to identify common sub-parts of the functions and extract these
into shared small helper functions this would make things easier ?



> 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.

Is there a need for this ?  That is - do we have users complaining that
the .octa pseudo-op is unable to handle unresolvable expressions ?  My
guess is that where this pseudo is used, it is always used with constants
or simple expressions that can be resolved at assembly time, and hence
no one has noticed the lack of relocation support.

Cheers
   Nick