Re: [PATCH 05/16] qemu/bswap: implement {ld,st}.*_p as functions

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/10/25 09:37, Richard Henderson wrote:
> On 3/10/25 09:14, Pierrick Bouvier wrote:
>> On 3/10/25 09:08, Richard Henderson wrote:
>>> On 3/9/25 21:58, Pierrick Bouvier wrote:
>>>> For now, they are duplicate of the same macros in cpu-all.h that we
>>>> eliminate in next commit.
>>>>
>>>> Keep code readable by not defining them with macros, but simply their
>>>> implementation.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> Why do you want these in bswap.h, rather than tswap.h?
>>> They're target swaps, after all.
>>>
>>>
>>> r~
>>
>> No preference on that, I simply added them to the same file than their explicit endianness
>> variant. Would you prefer the endianness agnostic variant to be in tswap.h instead?
> 
> I think I would.

Ok, I will move it.

> 
> In addition, I think we want
> 
> #ifdef COMPILING_PER_TARGET
> #define target_words_bigendian()  TARGET_BIG_ENDIAN
> #else
> bool target_words_bigendian(void);
> #endif
> 
> moving the conditional from around target_needs_bswap just below.
> 
> With that, we eliminate the extra branch that you're otherwise
> adding to target-specific code with this patch.
> 

I understand the change requested, but should we really aim in that 
direction? In the end, if we pursue the compilation units deduplication, 
the branch will be present anyway.

I'm ok with your change, just asking if we really want to preserve 
target specific code until the "end".

> 
> r~