On 4/16/25 17:18, Richard Henderson wrote:
On 4/16/25 16:38, Pierrick Bouvier wrote:
The only concern I have is that we could create silent "performance" related
bugs, where a
specific feature is deactivated because of a bad combination, but it's inherent
to this
approach and not a blocker.
I think I know what you mean, and the way I see things it that the silent
performance bug
was previously scattered across different sections of the code, whereas now it
is on the
same page. But underneath there is no real change.
Unless you mean something different?
It should be functionnally equivalent indeed, but in case one of cset_*
function contains bug, it might silently fallback to a slower
implementation. The TCG_TARGET_HAS_* were less error prone I guess, as
it's just a declaration.
But overall, the new approach is really better, so it's worth the risk.
As a more general question, how do you approach testing for a series like this
one? I see
two different challenges, as it touches the IR itself, and the various backends.
- For the IR, I don't know how extensive our complete test suite is (regarding
coverage of
all existing TCG ops), but I guess there are some holes there. It would be
interesting to
generate coverage data once we can get a single binary in the future.
I don't use anything more than our testsuite.
Coverage data would indeed be interesting; I've not attempted that.
I tried previously, but since we have duplicated compilation units per
target, this is confusing for any coverage tool, as soon as you try to
aggregate data from several targets.
- For the various backends:
* Are you able to compile QEMU on all concerned hosts and run testing there?
I have aarch64, arm, s390x via *.ci.qemu.org;
loongarch64, riscv64, ppc64le via the gcc compile farm.
* Or do you cross compile and run binaries emulated?
This is my only option for mipsel, mips64el.
I do not even have a cross-compile solution for ppc32, as there is no longer
any distro
support. I have been ignoring that, waiting to remove it when all 32-bit hosts
get kicked.
r~
