On 3/10/25 13:17, BALATON Zoltan wrote:
On Mon, 10 Mar 2025, Pierrick Bouvier wrote:
On 3/10/25 09:53, Richard Henderson wrote:
On 3/10/25 09:43, Pierrick Bouvier wrote:
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.bouv...@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".

All of target/ is target specific.  De-duplication will not eliminate that.


My vocabulary was wrong here. I meant "if we want to preserve target specific
macros" until the end.
Sure, there will always be compilation units (devices, cpus, helpers, ...)
specific to a target. I just wonder if sticking to ifdef paradigm for this
kind of code is worth the "optimization" we are supposed to get.

I've already tried to say that in the previous reply but maybe I can
explain it better here. I think keeping per target binaries would be
desired so single binary would not replace it just become an additional
option. For example when I want to play with old stuff I compile with
--target-list=ppc-softmmu and don't want to wait compiling all the other
targets I don't use and not even interested in PPC64. A distro may want to
ship a single qemu-system binary instead but other distros may prefer per
target packages not one huge package so users can decide which ones to
install. All of these are valid use cases, therefore this single binary
should be an additional option not the only true way from now on
replacing existing per target builds.


This valid use case is and *will* stay the default. There is no secret evil plan to break people habits here :).

For now, we don't even introduce a "single binary" configure option, simply because the resulting code could not link, so there is no point.

If you're curious about this, it is the command we use to check the work left before being able to do it:
# configure with any target list, we use all in our case
$ ./configure ...
$ jq --raw-output < build/compile_commands.json '.[].file' | sort |
  uniq -c | sort -rn | grep -v '^\s*1 '

Once this list is empty, we'll be able to link and execute a single binary (and adding a additional new main() for that). But first things first.

Regards,
BALATON Zoltan

I'll add the change requested.


r~



Reply via email to