[Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Alex Bennée]

The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1

for you to fetch changes up to f722d6877d4e91e984480a4b687cf12971b6c931:

  tests/Makfile: add check-softfloat rule (2019-01-17 09:12:33 +0000)

----------------------------------------------------------------
Some more softfloat/fpu fixes

   - make check-softfloat
   - fixes for fp-bench
   - workaround broken host fma
   - compile fix for s390x/clang

----------------------------------------------------------------
Alex Bennée (3):
      tests/Makefile: add floating point tests
      scripts/archive-source: include softfloat tests
      tests/Makfile: add check-softfloat rule

Emilio G. Cota (3):
      fp-bench: fix update_random_ops
      fp-bench: remove wrong exponent raise in fill_random
      softfloat: enforce softfloat if the host's FMA is broken

Thomas Huth (1):
      include/fpu/softfloat: Fix compilation with Clang on s390x

 fpu/softfloat.c                |  33 +++++++++++
 include/fpu/softfloat-macros.h |   2 +-
 scripts/archive-source.sh      |   2 +-
 tests/Makefile.include         | 132 ++++++++++++++++++++++++++++++++++++++++-
 tests/fp/fp-bench.c            |  15 ++---
 5 files changed, 173 insertions(+), 11 deletions(-)

-- 
2.17.1

[Qemu-devel] [PULL 1/7] fp-bench: fix update_random_ops

[Alex Bennée]

From: "Emilio G. Cota" <cota@braap.org>

The second test in the branches is wrong; fix while converting
to a switch statement, which is easier to get right.

Signed-off-by: Emilio G. Cota <cota@braap.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c
index f5bc5edebf..546bac9c9c 100644
--- a/tests/fp/fp-bench.c
+++ b/tests/fp/fp-bench.c
@@ -143,15 +143,20 @@ static void update_random_ops(int n_ops, enum precision prec)
     for (i = 0; i < n_ops; i++) {
         uint64_t r = random_ops[i];
 
-        if (prec == PREC_SINGLE || PREC_FLOAT32) {
+        switch (prec) {
+        case PREC_SINGLE:
+        case PREC_FLOAT32:
             do {
                 r = xorshift64star(r);
             } while (!float32_is_normal(r));
-        } else if (prec == PREC_DOUBLE || PREC_FLOAT64) {
+            break;
+        case PREC_DOUBLE:
+        case PREC_FLOAT64:
             do {
                 r = xorshift64star(r);
             } while (!float64_is_normal(r));
-        } else {
+            break;
+        default:
             g_assert_not_reached();
         }
         random_ops[i] = r;
-- 
2.17.1

[Qemu-devel] [PULL 2/7] fp-bench: remove wrong exponent raise in fill_random

[Alex Bennée]

From: "Emilio G. Cota" <cota@braap.org>

At this point random_ops[] only contains normals, so there's
no need to do anything to them. In fact, raising the exponent
here can make the output !normal, which is precisely
what the comment says we want to avoid.

Signed-off-by: Emilio G. Cota <cota@braap.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c
index 546bac9c9c..4ba5e1d2d4 100644
--- a/tests/fp/fp-bench.c
+++ b/tests/fp/fp-bench.c
@@ -176,8 +176,6 @@ static void fill_random(union fp *ops, int n_ops, enum precision prec,
             if (no_neg && float32_is_neg(ops[i].f32)) {
                 ops[i].f32 = float32_chs(ops[i].f32);
             }
-            /* raise the exponent to limit the frequency of denormal results */
-            ops[i].f32 |= 0x40000000;
             break;
         case PREC_DOUBLE:
         case PREC_FLOAT64:
@@ -185,8 +183,6 @@ static void fill_random(union fp *ops, int n_ops, enum precision prec,
             if (no_neg && float64_is_neg(ops[i].f64)) {
                 ops[i].f64 = float64_chs(ops[i].f64);
             }
-            /* raise the exponent to limit the frequency of denormal results */
-            ops[i].f64 |= LIT64(0x4000000000000000);
             break;
         default:
             g_assert_not_reached();
-- 
2.17.1

[Qemu-devel] [PULL 3/7] softfloat: enforce softfloat if the host's FMA is broken

[Alex Bennée]

From: "Emilio G. Cota" <cota@braap.org>

The added branch to the FMA ops is marked as unlikely and therefore
its impact on performance (measured with fp-bench) is within noise range
when measured on an Intel(R) Xeon(R) Gold 6142 CPU @ 2.60GHz.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 59eac97d10..9132d7a0b0 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1542,6 +1542,8 @@ soft_f64_muladd(float64 a, float64 b, float64 c, int flags,
     return float64_round_pack_canonical(pr, status);
 }
 
+static bool force_soft_fma;
+
 float32 QEMU_FLATTEN
 float32_muladd(float32 xa, float32 xb, float32 xc, int flags, float_status *s)
 {
@@ -1562,6 +1564,11 @@ float32_muladd(float32 xa, float32 xb, float32 xc, int flags, float_status *s)
     if (unlikely(!f32_is_zon3(ua, ub, uc))) {
         goto soft;
     }
+
+    if (unlikely(force_soft_fma)) {
+        goto soft;
+    }
+
     /*
      * When (a || b) == 0, there's no need to check for under/over flow,
      * since we know the addend is (normal || 0) and the product is 0.
@@ -1623,6 +1630,11 @@ float64_muladd(float64 xa, float64 xb, float64 xc, int flags, float_status *s)
     if (unlikely(!f64_is_zon3(ua, ub, uc))) {
         goto soft;
     }
+
+    if (unlikely(force_soft_fma)) {
+        goto soft;
+    }
+
     /*
      * When (a || b) == 0, there's no need to check for under/over flow,
      * since we know the addend is (normal || 0) and the product is 0.
@@ -7974,3 +7986,24 @@ float128 float128_scalbn(float128 a, int n, float_status *status)
                                          , status);
 
 }
+
+static void __attribute__((constructor)) softfloat_init(void)
+{
+    union_float64 ua, ub, uc, ur;
+
+    if (QEMU_NO_HARDFLOAT) {
+        return;
+    }
+    /*
+     * Test that the host's FMA is not obviously broken. For example,
+     * glibc < 2.23 can perform an incorrect FMA on certain hosts; see
+     *   https://sourceware.org/bugzilla/show_bug.cgi?id=13304
+     */
+    ua.s = 0x0020000000000001ULL;
+    ub.s = 0x3ca0000000000000ULL;
+    uc.s = 0x0020000000000000ULL;
+    ur.h = fma(ua.h, ub.h, uc.h);
+    if (ur.s != 0x0020000000000001ULL) {
+        force_soft_fma = true;
+    }
+}
-- 
2.17.1

[Qemu-devel] [PULL 4/7] include/fpu/softfloat: Fix compilation with Clang on s390x

[Alex Bennée]

From: Thomas Huth <thuth@redhat.com>

Clang v7.0.1 does not like the __int128 variable type for inline
assembly on s390x:

In file included from fpu/softfloat.c:97:
include/fpu/softfloat-macros.h:647:9: error: inline asm error:
 This value type register class is not natively supported!
    asm("dlgr %0, %1" : "+r"(n) : "r"(d));
        ^

Disable this code part there now when compiling with Clang, so that
the generic code gets used instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index b1d772e6d4..bd5b6418e3 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -641,7 +641,7 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
     uint64_t q;
     asm("divq %4" : "=a"(q), "=d"(*r) : "0"(n0), "1"(n1), "rm"(d));
     return q;
-#elif defined(__s390x__)
+#elif defined(__s390x__) && !defined(__clang__)
     /* Need to use a TImode type to get an even register pair for DLGR.  */
     unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
     asm("dlgr %0, %1" : "+r"(n) : "r"(d));
-- 
2.17.1

[Qemu-devel] [PULL 5/7] tests/Makefile: add floating point tests

[Alex Bennée]

Wire up test/fp-test into the main testing Makefile. Currently we skip
some of the extF80 and f128 related tests. Once we re-factor and fix
these tests the plumbing should get simpler.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/tests/Makefile.include b/tests/Makefile.include
index f403a6571d..43888f3ad5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -878,6 +878,121 @@ check-report-unit.tap: $(check-unit-y)
 check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check-report-unit.tap
 	$(call quiet-command,./scripts/tap-merge.py $^ > $@,"GEN","$@")
 
+# FPU Emulation tests (aka softfloat)
+#
+# As we still have some places that need fixing the rules are a little
+# more complex than they need to be and have to override some of the
+# generic Makefile expansions. Once we are cleanly passing all
+# the tests we can simplify the make syntax.
+
+# the build dir is created by configure
+$(BUILD_DIR)/tests/fp/fp-test: $(BUILD_DIR)/tests/fp
+	$(call quiet-command, \
+	  cd $(BUILD_DIR)/tests/fp && make, \
+          "BUILD", $<)
+
+# The full test suite can take a bit of time, default to a quick run
+ifeq ($(SPEED), quick)
+FP_TL=-l 1
+else
+FP_TL=-l 2 -r all
+endif
+
+# $1 = tests, $2 = description
+test-softfloat = $(call quiet-command, \
+			cd $(BUILD_DIR)/tests/fp && ./fp-test -s $(FP_TL) $1 > $2.out 2> $2.err, \
+			"FLOAT TEST", $2)
+
+# Conversion Routines:
+# FIXME: i32_to_extF80 (broken), i64_to_extF80 (broken)
+#        ui32_to_f128 (not implemented), f128_to_ui32 (not implemented)
+#        extF80_roundToInt (broken)
+#
+check-softfloat-conv: tests/fp/fp-test
+	$(call test-softfloat, \
+		i32_to_f16 i64_to_f16 \
+		i32_to_f32 i64_to_f32 \
+		i32_to_f64 i64_to_f64 \
+		i32_to_f128 i64_to_f128, int-to-float)
+	$(call test-softfloat, \
+		ui32_to_f16 ui64_to_f16 \
+		ui32_to_f32 ui64_to_f32 \
+		ui32_to_f64 ui64_to_f64 \
+		ui64_to_f128, uint-to-float)
+	$(call test-softfloat, \
+		f16_to_i32 f16_to_i32_r_minMag \
+		f32_to_i32 f32_to_i32_r_minMag \
+		f64_to_i32 f64_to_i32_r_minMag \
+		extF80_to_i32 extF80_to_i32_r_minMag \
+		f128_to_i32 f128_to_i32_r_minMag \
+		f16_to_i64 f16_to_i64_r_minMag \
+		f32_to_i64 f32_to_i64_r_minMag \
+		f64_to_i64 f64_to_i64_r_minMag \
+		extF80_to_i64 extF80_to_i64_r_minMag \
+		f128_to_i64 f128_to_i64_r_minMag, \
+		float-to-int)
+	$(call test-softfloat, \
+		f16_to_ui32 f16_to_ui32_r_minMag \
+		f32_to_ui32 f32_to_ui32_r_minMag \
+		f64_to_ui32 f64_to_ui32_r_minMag \
+		f16_to_ui64 f16_to_ui64_r_minMag \
+		f32_to_ui64 f32_to_ui64_r_minMag \
+		f64_to_ui64 f64_to_ui64_r_minMag, \
+		float-to-uint)
+	$(call test-softfloat, \
+		f16_roundToInt f32_roundToInt \
+		f64_roundToInt f128_roundToInt, \
+		round-to-integer)
+
+# Generic rule for all float operations
+#
+# Some patterns are overidden due to broken or missing tests.
+# Hopefully these can be removed over time.
+
+check-softfloat-%: tests/fp/fp-test
+	$(call test-softfloat, f16_$* f32_$* f64_$* extF80_$* f128_$*, $*)
+
+# Float Compare routines
+SF_COMPARE_OPS=eq eq_signaling le le_quiet lt_quiet
+SF_COMPARE_RULES=$(patsubst %,check-softfloat-%, $(SF_COMPARE_OPS))
+
+# FIXME: extF80_le_quiet (broken)
+check-softfloat-le_quiet: tests/fp/fp-test
+	$(call test-softfloat, 				\
+		f16_le_quiet f32_le_quiet f64_le_quiet  \
+		f128_le_quiet, 				\
+		le_quiet)
+
+# FIXME: extF80_lt_quiet (broken)
+check-softfloat-lt_quiet: tests/fp/fp-test
+	$(call test-softfloat, 				\
+		f16_lt_quiet f32_lt_quiet f64_lt_quiet  \
+		f128_lt_quiet, 				\
+		lt_quiet)
+
+.PHONY: check-softfloat-compare
+check-softfloat-compare: $(SF_COMPARE_RULES)
+
+# Math Operations
+
+# FIXME: extF80_mulAdd (missing)
+check-softfloat-mulAdd: tests/fp/fp-test
+	$(call test-softfloat, \
+		f16_mulAdd f32_mulAdd f64_mulAdd f128_mulAdd, \
+		mulAdd)
+
+# FIXME: extF80_rem (broken)
+check-softfloat-rem: tests/fp/fp-test
+	$(call test-softfloat, \
+		f16_rem f32_rem f64_rem f128_rem, \
+		rem)
+
+SF_MATH_OPS=add sub mul mulAdd div rem sqrt
+SF_MATH_RULES=$(patsubst %,check-softfloat-%, $(SF_MATH_OPS))
+
+.PHONY: check-softfloat-ops
+check-softfloat-ops: $(SF_MATH_RULES)
+
 # Per guest TCG tests
 
 LINUX_USER_TARGETS=$(filter %-linux-user,$(TARGET_DIRS))
-- 
2.17.1

[Qemu-devel] [PULL 6/7] scripts/archive-source: include softfloat tests

[Alex Bennée]

We need these if we want to run unit/softfloat tests in our docker
containers.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 62bd22578b..6eed2a29bd 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -26,7 +26,7 @@ vroot_dir="${tar_file}.vroot"
 # independent of what the developer currently has initialized
 # in their checkout, because the build environment is completely
 # different to the host OS.
-submodules="dtc ui/keycodemapdb"
+submodules="dtc ui/keycodemapdb tests/fp/berkeley-softfloat-3 tests/fp/berkeley-testfloat-3"
 
 trap "status=$?; rm -rf \"$list_file\" \"$vroot_dir\"; exit \$status" 0 1 2 3 15
 
-- 
2.17.1

[Qemu-devel] [PULL 7/7] tests/Makfile: add check-softfloat rule

[Alex Bennée]

This adds a rule to run all of our softfloat tests. It is included as
a pre-requisite to check-tcg and check-unit as well.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 43888f3ad5..1de4f1b7dd 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -11,6 +11,7 @@ check-help:
 	@echo " $(MAKE) check-qapi-schema    Run QAPI schema tests"
 	@echo " $(MAKE) check-block          Run block tests"
 	@echo " $(MAKE) check-tcg            Run TCG tests"
+	@echo " $(MAKE) check-softfloat      Run FPU emulation tests"
 	@echo " $(MAKE) check-acceptance     Run all acceptance (functional) tests"
 	@echo " $(MAKE) check-report.html    Generates an HTML test report"
 	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
@@ -993,6 +994,18 @@ SF_MATH_RULES=$(patsubst %,check-softfloat-%, $(SF_MATH_OPS))
 .PHONY: check-softfloat-ops
 check-softfloat-ops: $(SF_MATH_RULES)
 
+# Finally a generic rule to test all of softfoat. If TCG isnt't
+# enabled we define a null operation which skips the tests.
+
+.PHONY: check-softfloat
+ifeq ($(CONFIG_TCG),y)
+check-softfloat: check-softfloat-conv check-softfloat-compare check-softfloat-ops
+else
+check-softfloat:
+	$(call quiet-command, /bin/true, "FLOAT TEST", \
+		"SKIPPED for non-TCG builds")
+endif
+
 # Per guest TCG tests
 
 LINUX_USER_TARGETS=$(filter %-linux-user,$(TARGET_DIRS))
@@ -1025,7 +1038,7 @@ clean-tcg-tests-%:
 build-tcg: $(BUILD_TCG_TARGET_RULES)
 
 .PHONY: check-tcg
-check-tcg: $(RUN_TCG_TARGET_RULES)
+check-tcg: check-softfloat $(RUN_TCG_TARGET_RULES)
 
 .PHONY: clean-tcg
 clean-tcg: $(CLEAN_TCG_TARGET_RULES)
@@ -1107,7 +1120,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-qapi-schema check-unit check-qtest check-decodetree
+check: check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
 	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
-- 
2.17.1

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Peter Maydell]

On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
>
> for you to fetch changes up to f722d6877d4e91e984480a4b687cf12971b6c931:
>
>   tests/Makfile: add check-softfloat rule (2019-01-17 09:12:33 +0000)
>
> ----------------------------------------------------------------
> Some more softfloat/fpu fixes
>
>    - make check-softfloat
>    - fixes for fp-bench
>    - workaround broken host fma
>    - compile fix for s390x/clang
>
> ----------------------------------------------------------------

FreeBSD, OSX, x86-64 Linux clang builds:

Lots of these trying to build code in tests/fp:

warning: unknown warning option '-Wno-discarded-qualifiers'; did you
mean '-Wno-ignored-qualifiers'? [-Wunknown-warning-option]
warning: unknown warning option '-Wno-maybe-uninitialized'; did you
mean '-Wno-uninitialized'? [-Wunknown-warning-option]


/var/tmp/qemu-test.Z0AmK8/tests/fp/berkeley-testfloat-3/source/fail.c:44:7:
warning: initializing 'char *' with an expression of type 'const char
[1]' discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]
char *fail_programName = "";
      ^                  ~~


fp-test.c:905:33: warning: incompatible pointer types assigning to
'uint_fast8_t (*)(void)' (aka 'unsigned int (*)(void)') from 'uint8_t
(void)' (aka 'unsigned char (void)') [-Wincompatible-pointer-types]
    testLoops_trueFlagsFunction = slow_clear_flags;
                                ^ ~~~~~~~~~~~~~~~~
fp-test.c:906:33: warning: incompatible pointer types assigning to
'uint_fast8_t (*)(void)' (aka 'unsigned int (*)(void)') from 'uint8_t
(void)' (aka 'unsigned char (void)') [-Wincompatible-pointer-types]
    testLoops_subjFlagsFunction = qemu_clear_flags;
                                ^ ~~~~~~~~~~~~~~~~


PPC64, AArch64:
 * same warning about discarding 'const' in fail.c as above
 * cc1: warning: unrecognized command line option
"-Wno-discarded-qualifiers" [enabled by default]

NetBSD:
 * same warning about slow_clear_flags/qemu_clear_flags assignments

S390X host:
Looks like a failure running the tests, but no diagnostics about
what exactly went wrong or clear "test failed" indicator:

cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
i32_to_f16 i64_to_f16 i32_to_f32 i64_t
o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
int-to-float.out 2>  int-to-float.err
/home/linux1/qemu/tests/Makefile.include:913: recipe for target
'check-softfloat-conv' failed
make: *** [check-softfloat-conv] Error 1

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Emilio G. Cota]

On Thu, Jan 17, 2019 at 17:37:54 +0000, Peter Maydell wrote:
> On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
> >
> >   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
(snip)
> 
> FreeBSD, OSX, x86-64 Linux clang builds:
(snip)
> PPC64, AArch64:
(snip)
> NetBSD:
(snip)

I have added a few commits to fix these -- diff below.
Note that one fix requires a small change in the testfloat
submodule, which I'm pulling here.

Alex, can you cherry-pick the commits in this branch?

  https://github.com/cota/qemu/tree/bennee-pull

They should go in before adding fp-test to the automated build,
of course. I left them at the top to make it easier for
you to cherry-pick.

Also, since this will require a respin, you might want to fix the
"Makfile" typo in patch 7's title.

> S390X host:
> Looks like a failure running the tests, but no diagnostics about
> what exactly went wrong or clear "test failed" indicator:
> 
> cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
> i32_to_f16 i64_to_f16 i32_to_f32 i64_t
> o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
> int-to-float.out 2>  int-to-float.err
> /home/linux1/qemu/tests/Makefile.include:913: recipe for target
> 'check-softfloat-conv' failed
> make: *** [check-softfloat-conv] Error 1

What are the contents of "int-to-float.err"?

Thanks,

		Emilio

---
diff --git a/tests/fp/Makefile b/tests/fp/Makefile
index 5019dcdca0..5a35e7c210 100644
--- a/tests/fp/Makefile
+++ b/tests/fp/Makefile
@@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
 TF_CFLAGS :=
 TF_CFLAGS += -Wno-strict-prototypes
 TF_CFLAGS += -Wno-unknown-pragmas
-TF_CFLAGS += -Wno-discarded-qualifiers
-TF_CFLAGS += -Wno-maybe-uninitialized
+TF_CFLAGS += -Wno-uninitialized
 TF_CFLAGS += -Wno-missing-prototypes
 TF_CFLAGS += -Wno-return-type
 TF_CFLAGS += -Wno-unused-function
diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
index ca9fa2ba05..5a59dcec19 160000
--- a/tests/fp/berkeley-testfloat-3
+++ b/tests/fp/berkeley-testfloat-3
@@ -1 +1 @@
-Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
+Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
index fca576309c..2a35ef601d 100644
--- a/tests/fp/fp-test.c
+++ b/tests/fp/fp-test.c
@@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
     return 0;
 }

-static uint8_t slow_clear_flags(void)
+static uint_fast8_t slow_clear_flags(void)
 {
     uint8_t prev = slowfloat_exceptionFlags;

@@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
     return prev;
 }

-static uint8_t qemu_clear_flags(void)
+static uint_fast8_t qemu_clear_flags(void)
 {
     uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Peter Maydell]

On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
>
> On Thu, Jan 17, 2019 at 17:37:54 +0000, Peter Maydell wrote:
> > On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
> > >
> > > The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
> > >
> > >   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
> (snip)
> >
> > FreeBSD, OSX, x86-64 Linux clang builds:
> (snip)
> > PPC64, AArch64:
> (snip)
> > NetBSD:
> (snip)
>
> I have added a few commits to fix these -- diff below.
> Note that one fix requires a small change in the testfloat
> submodule, which I'm pulling here.
>
> Alex, can you cherry-pick the commits in this branch?
>
>   https://github.com/cota/qemu/tree/bennee-pull
>
> They should go in before adding fp-test to the automated build,
> of course. I left them at the top to make it easier for
> you to cherry-pick.
>
> Also, since this will require a respin, you might want to fix the
> "Makfile" typo in patch 7's title.
>
> > S390X host:
> > Looks like a failure running the tests, but no diagnostics about
> > what exactly went wrong or clear "test failed" indicator:
> >
> > cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
> > i32_to_f16 i64_to_f16 i32_to_f32 i64_t
> > o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
> > int-to-float.out 2>  int-to-float.err
> > /home/linux1/qemu/tests/Makefile.include:913: recipe for target
> > 'check-softfloat-conv' failed
> > make: *** [check-softfloat-conv] Error 1
>
> What are the contents of "int-to-float.err"?

linux1@lxub05:~$ cat qemu/build/all/tests/fp/int-to-float.err
>> Testing i32_to_f16, rounding near_even
372 tests total.
372 tests performed.
>> Testing i64_to_f16, rounding near_even
756 tests total.
756 tests performed.
>> Testing i32_to_f32, rounding near_even
372 tests total.
372 tests performed.
>> Testing i64_to_f32, rounding near_even
756 tests total.
756 tests performed.
>> Testing i32_to_f64
372 tests total.
372 tests performed.
>> Testing i64_to_f64, rounding near_even
756 tests total.
756 tests performed.
>> Testing i32_to_f128
372 tests total.
21 tests performed; 20 errors found.


> diff --git a/tests/fp/Makefile b/tests/fp/Makefile
> index 5019dcdca0..5a35e7c210 100644
> --- a/tests/fp/Makefile
> +++ b/tests/fp/Makefile
> @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
>  TF_CFLAGS :=
>  TF_CFLAGS += -Wno-strict-prototypes
>  TF_CFLAGS += -Wno-unknown-pragmas
> -TF_CFLAGS += -Wno-discarded-qualifiers
> -TF_CFLAGS += -Wno-maybe-uninitialized
> +TF_CFLAGS += -Wno-uninitialized
>  TF_CFLAGS += -Wno-missing-prototypes
>  TF_CFLAGS += -Wno-return-type
>  TF_CFLAGS += -Wno-unused-function

configure has logic to check whether it can use particular
warning enable/disable flags. Newer gcc (and I hope clang
but forget) will happily silently allow -Wno-random-new-thing
even if they don't support -Wrandom-new-thing) but I'm not
sure our minimum compiler version is yet new enough to
be able to rely on that (indeed the warning messages suggest
it is not).

> diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
> index ca9fa2ba05..5a59dcec19 160000
> --- a/tests/fp/berkeley-testfloat-3
> +++ b/tests/fp/berkeley-testfloat-3
> @@ -1 +1 @@
> -Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
> +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
> diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
> index fca576309c..2a35ef601d 100644
> --- a/tests/fp/fp-test.c
> +++ b/tests/fp/fp-test.c
> @@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
>      return 0;
>  }
>
> -static uint8_t slow_clear_flags(void)
> +static uint_fast8_t slow_clear_flags(void)
>  {
>      uint8_t prev = slowfloat_exceptionFlags;
>
> @@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
>      return prev;
>  }
>
> -static uint8_t qemu_clear_flags(void)
> +static uint_fast8_t qemu_clear_flags(void)
>  {
>      uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);

Why are we using uint_fast8_t here anyway? We switched
softfloat to using plain old uint8_t everywhere a while
back.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Emilio G. Cota]

On Thu, Jan 17, 2019 at 18:55:33 +0000, Peter Maydell wrote:
> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
> > What are the contents of "int-to-float.err"?
> 
> linux1@lxub05:~$ cat qemu/build/all/tests/fp/int-to-float.err
> >> Testing i32_to_f16, rounding near_even
> 372 tests total.
> 372 tests performed.
> >> Testing i64_to_f16, rounding near_even
> 756 tests total.
> 756 tests performed.
> >> Testing i32_to_f32, rounding near_even
> 372 tests total.
> 372 tests performed.
> >> Testing i64_to_f32, rounding near_even
> 756 tests total.
> 756 tests performed.
> >> Testing i32_to_f64
> 372 tests total.
> 372 tests performed.
> >> Testing i64_to_f64, rounding near_even
> 756 tests total.
> 756 tests performed.
> >> Testing i32_to_f128
> 372 tests total.
> 21 tests performed; 20 errors found.

I see, so i32_to_f128 is failing on this host. Is there
a s390x machine I could access? I don't see one in the
gcc compile farm.

> > diff --git a/tests/fp/Makefile b/tests/fp/Makefile
> > index 5019dcdca0..5a35e7c210 100644
> > --- a/tests/fp/Makefile
> > +++ b/tests/fp/Makefile
> > @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
> >  TF_CFLAGS :=
> >  TF_CFLAGS += -Wno-strict-prototypes
> >  TF_CFLAGS += -Wno-unknown-pragmas
> > -TF_CFLAGS += -Wno-discarded-qualifiers
> > -TF_CFLAGS += -Wno-maybe-uninitialized
> > +TF_CFLAGS += -Wno-uninitialized
> >  TF_CFLAGS += -Wno-missing-prototypes
> >  TF_CFLAGS += -Wno-return-type
> >  TF_CFLAGS += -Wno-unused-function
> 
> configure has logic to check whether it can use particular
> warning enable/disable flags. Newer gcc (and I hope clang
> but forget) will happily silently allow -Wno-random-new-thing
> even if they don't support -Wrandom-new-thing) but I'm not
> sure our minimum compiler version is yet new enough to
> be able to rely on that (indeed the warning messages suggest
> it is not).

I figured that -Wno-unitialized is documented to work on both
gcc and clang, and the change in testfloat drops the need
for no-discarded-qualifiers, so we can go with this without
messing with configure.

> > diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
> > index ca9fa2ba05..5a59dcec19 160000
> > --- a/tests/fp/berkeley-testfloat-3
> > +++ b/tests/fp/berkeley-testfloat-3
> > @@ -1 +1 @@
> > -Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
> > +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
> > diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
> > index fca576309c..2a35ef601d 100644
> > --- a/tests/fp/fp-test.c
> > +++ b/tests/fp/fp-test.c
> > @@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
> >      return 0;
> >  }
> >
> > -static uint8_t slow_clear_flags(void)
> > +static uint_fast8_t slow_clear_flags(void)
> >  {
> >      uint8_t prev = slowfloat_exceptionFlags;
> >
> > @@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
> >      return prev;
> >  }
> >
> > -static uint8_t qemu_clear_flags(void)
> > +static uint_fast8_t qemu_clear_flags(void)
> >  {
> >      uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);
> 
> Why are we using uint_fast8_t here anyway? We switched
> softfloat to using plain old uint8_t everywhere a while
> back.

berkeley-testfloat-3 uses uint_fast8_t extensively,
and these function signatures come from it, not from our
softfloat implementation.

Thanks,

		E.

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
>>
>> On Thu, Jan 17, 2019 at 17:37:54 +0000, Peter Maydell wrote:
>> > On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>> > >
>> > > The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
>> > >
>> > >   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
>> > >
>> > > are available in the Git repository at:
>> > >
>> > >   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
>> (snip)
>> >
>> > FreeBSD, OSX, x86-64 Linux clang builds:
>> (snip)
>> > PPC64, AArch64:
>> (snip)
>> > NetBSD:
>> (snip)
>>
>> I have added a few commits to fix these -- diff below.
>> Note that one fix requires a small change in the testfloat
>> submodule, which I'm pulling here.
>>
>> Alex, can you cherry-pick the commits in this branch?
>>
>>   https://github.com/cota/qemu/tree/bennee-pull
>>
>> They should go in before adding fp-test to the automated build,
>> of course. I left them at the top to make it easier for
>> you to cherry-pick.
>>
>> Also, since this will require a respin, you might want to fix the
>> "Makfile" typo in patch 7's title.
>>
>> > S390X host:
>> > Looks like a failure running the tests, but no diagnostics about
>> > what exactly went wrong or clear "test failed" indicator:
>> >
>> > cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
>> > i32_to_f16 i64_to_f16 i32_to_f32 i64_t
>> > o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
>> > int-to-float.out 2>  int-to-float.err
>> > /home/linux1/qemu/tests/Makefile.include:913: recipe for target
>> > 'check-softfloat-conv' failed
>> > make: *** [check-softfloat-conv] Error 1
>>
>> What are the contents of "int-to-float.err"?
>
<snip>
>>> Testing i32_to_f128
> 372 tests total.
> 21 tests performed; 20 errors found.

It's probably not a regression but a failure in the old f128 code. When
I was enabling the tests I basically turned them all on and then
disabled what failed (all of which was exercising non-refactored code).
But of course this was all on x86_64 (although I also ran them on a
aarch64 host). It looks like more errors come out on a s390x host for
some reason. You can run manually for a more complete test with more
coverage:

  ./fp-test -l 2 -r all i32_to_f128

>
>
>> diff --git a/tests/fp/Makefile b/tests/fp/Makefile
>> index 5019dcdca0..5a35e7c210 100644
>> --- a/tests/fp/Makefile
>> +++ b/tests/fp/Makefile
>> @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
>>  TF_CFLAGS :=
>>  TF_CFLAGS += -Wno-strict-prototypes
>>  TF_CFLAGS += -Wno-unknown-pragmas
>> -TF_CFLAGS += -Wno-discarded-qualifiers
>> -TF_CFLAGS += -Wno-maybe-uninitialized
>> +TF_CFLAGS += -Wno-uninitialized
>>  TF_CFLAGS += -Wno-missing-prototypes
>>  TF_CFLAGS += -Wno-return-type
>>  TF_CFLAGS += -Wno-unused-function
>
> configure has logic to check whether it can use particular
> warning enable/disable flags. Newer gcc (and I hope clang
> but forget) will happily silently allow -Wno-random-new-thing
> even if they don't support -Wrandom-new-thing) but I'm not
> sure our minimum compiler version is yet new enough to
> be able to rely on that (indeed the warning messages suggest
> it is not).
>
>> diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
>> index ca9fa2ba05..5a59dcec19 160000
>> --- a/tests/fp/berkeley-testfloat-3
>> +++ b/tests/fp/berkeley-testfloat-3
>> @@ -1 +1 @@
>> -Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
>> +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
>> diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
>> index fca576309c..2a35ef601d 100644
>> --- a/tests/fp/fp-test.c
>> +++ b/tests/fp/fp-test.c
>> @@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
>>      return 0;
>>  }
>>
>> -static uint8_t slow_clear_flags(void)
>> +static uint_fast8_t slow_clear_flags(void)
>>  {
>>      uint8_t prev = slowfloat_exceptionFlags;
>>
>> @@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
>>      return prev;
>>  }
>>
>> -static uint8_t qemu_clear_flags(void)
>> +static uint_fast8_t qemu_clear_flags(void)
>>  {
>>      uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);
>
> Why are we using uint_fast8_t here anyway? We switched
> softfloat to using plain old uint8_t everywhere a while
> back.
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Philippe Mathieu-Daudé]

Hi Alex,

On 1/17/19 9:10 PM, Alex Bennée wrote:
> 
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
>>>
>>> On Thu, Jan 17, 2019 at 17:37:54 +0000, Peter Maydell wrote:
>>>> On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>>>
>>>>> The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
>>>>>
>>>>>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
>>>>>
>>>>> are available in the Git repository at:
>>>>>
>>>>>   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
>>> (snip)
>>>>
>>>> FreeBSD, OSX, x86-64 Linux clang builds:
>>> (snip)
>>>> PPC64, AArch64:
>>> (snip)
>>>> NetBSD:
>>> (snip)
>>>
>>> I have added a few commits to fix these -- diff below.
>>> Note that one fix requires a small change in the testfloat
>>> submodule, which I'm pulling here.
>>>
>>> Alex, can you cherry-pick the commits in this branch?
>>>
>>>   https://github.com/cota/qemu/tree/bennee-pull
>>>
>>> They should go in before adding fp-test to the automated build,
>>> of course. I left them at the top to make it easier for
>>> you to cherry-pick.
>>>
>>> Also, since this will require a respin, you might want to fix the
>>> "Makfile" typo in patch 7's title.
>>>
>>>> S390X host:
>>>> Looks like a failure running the tests, but no diagnostics about
>>>> what exactly went wrong or clear "test failed" indicator:
>>>>
>>>> cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
>>>> i32_to_f16 i64_to_f16 i32_to_f32 i64_t
>>>> o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
>>>> int-to-float.out 2>  int-to-float.err
>>>> /home/linux1/qemu/tests/Makefile.include:913: recipe for target
>>>> 'check-softfloat-conv' failed
>>>> make: *** [check-softfloat-conv] Error 1
>>>
>>> What are the contents of "int-to-float.err"?

Can you modify the Makefile.include to use:
 "fp-test ... 2>int-to-float.err || {cat int-to-float.err && exit 1;}"

Thanks!

Phil.

>>
> <snip>
>>>> Testing i32_to_f128
>> 372 tests total.
>> 21 tests performed; 20 errors found.
> 
> It's probably not a regression but a failure in the old f128 code. When
> I was enabling the tests I basically turned them all on and then
> disabled what failed (all of which was exercising non-refactored code).
> But of course this was all on x86_64 (although I also ran them on a
> aarch64 host). It looks like more errors come out on a s390x host for
> some reason. You can run manually for a more complete test with more
> coverage:
> 
>   ./fp-test -l 2 -r all i32_to_f128
> 
>>
>>
>>> diff --git a/tests/fp/Makefile b/tests/fp/Makefile
>>> index 5019dcdca0..5a35e7c210 100644
>>> --- a/tests/fp/Makefile
>>> +++ b/tests/fp/Makefile
>>> @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
>>>  TF_CFLAGS :=
>>>  TF_CFLAGS += -Wno-strict-prototypes
>>>  TF_CFLAGS += -Wno-unknown-pragmas
>>> -TF_CFLAGS += -Wno-discarded-qualifiers
>>> -TF_CFLAGS += -Wno-maybe-uninitialized
>>> +TF_CFLAGS += -Wno-uninitialized
>>>  TF_CFLAGS += -Wno-missing-prototypes
>>>  TF_CFLAGS += -Wno-return-type
>>>  TF_CFLAGS += -Wno-unused-function
>>
>> configure has logic to check whether it can use particular
>> warning enable/disable flags. Newer gcc (and I hope clang
>> but forget) will happily silently allow -Wno-random-new-thing
>> even if they don't support -Wrandom-new-thing) but I'm not
>> sure our minimum compiler version is yet new enough to
>> be able to rely on that (indeed the warning messages suggest
>> it is not).
>>
>>> diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
>>> index ca9fa2ba05..5a59dcec19 160000
>>> --- a/tests/fp/berkeley-testfloat-3
>>> +++ b/tests/fp/berkeley-testfloat-3
>>> @@ -1 +1 @@
>>> -Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
>>> +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
>>> diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
>>> index fca576309c..2a35ef601d 100644
>>> --- a/tests/fp/fp-test.c
>>> +++ b/tests/fp/fp-test.c
>>> @@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
>>>      return 0;
>>>  }
>>>
>>> -static uint8_t slow_clear_flags(void)
>>> +static uint_fast8_t slow_clear_flags(void)
>>>  {
>>>      uint8_t prev = slowfloat_exceptionFlags;
>>>
>>> @@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
>>>      return prev;
>>>  }
>>>
>>> -static uint8_t qemu_clear_flags(void)
>>> +static uint_fast8_t qemu_clear_flags(void)
>>>  {
>>>      uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);
>>
>> Why are we using uint_fast8_t here anyway? We switched
>> softfloat to using plain old uint8_t everywhere a while
>> back.
>>
>> thanks
>> -- PMM
> 
> 
> --
> Alex Bennée
> 

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Hi Alex,
>
> On 1/17/19 9:10 PM, Alex Bennée wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
>>>>
>>>> On Thu, Jan 17, 2019 at 17:37:54 +0000, Peter Maydell wrote:
>>>>> On Thu, 17 Jan 2019 at 13:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>>>>
>>>>>> The following changes since commit 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc:
>>>>>>
>>>>>>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-15 17:24:00 +0000)
>>>>>>
>>>>>> are available in the Git repository at:
>>>>>>
>>>>>>   https://github.com/stsquad/qemu.git tags/pull-fpu-next-170119-1
>>>> (snip)
>>>>>
>>>>> FreeBSD, OSX, x86-64 Linux clang builds:
>>>> (snip)
>>>>> PPC64, AArch64:
>>>> (snip)
>>>>> NetBSD:
>>>> (snip)
>>>>
>>>> I have added a few commits to fix these -- diff below.
>>>> Note that one fix requires a small change in the testfloat
>>>> submodule, which I'm pulling here.
>>>>
>>>> Alex, can you cherry-pick the commits in this branch?
>>>>
>>>>   https://github.com/cota/qemu/tree/bennee-pull
>>>>
>>>> They should go in before adding fp-test to the automated build,
>>>> of course. I left them at the top to make it easier for
>>>> you to cherry-pick.
>>>>
>>>> Also, since this will require a respin, you might want to fix the
>>>> "Makfile" typo in patch 7's title.
>>>>
>>>>> S390X host:
>>>>> Looks like a failure running the tests, but no diagnostics about
>>>>> what exactly went wrong or clear "test failed" indicator:
>>>>>
>>>>> cd /home/linux1/qemu/build/all/tests/fp && ./fp-test -s -l 1
>>>>> i32_to_f16 i64_to_f16 i32_to_f32 i64_t
>>>>> o_f32 i32_to_f64 i64_to_f64 i32_to_f128 i64_to_f128 >
>>>>> int-to-float.out 2>  int-to-float.err
>>>>> /home/linux1/qemu/tests/Makefile.include:913: recipe for target
>>>>> 'check-softfloat-conv' failed
>>>>> make: *** [check-softfloat-conv] Error 1
>>>>
>>>> What are the contents of "int-to-float.err"?
>
> Can you modify the Makefile.include to use:
>  "fp-test ... 2>int-to-float.err || {cat int-to-float.err && exit 1;}"

Well it's in the test-softfloat macro but sure...

>
> Thanks!
>
> Phil.
>
>>>
>> <snip>
>>>>> Testing i32_to_f128
>>> 372 tests total.
>>> 21 tests performed; 20 errors found.
>>
>> It's probably not a regression but a failure in the old f128 code. When
>> I was enabling the tests I basically turned them all on and then
>> disabled what failed (all of which was exercising non-refactored code).
>> But of course this was all on x86_64 (although I also ran them on a
>> aarch64 host). It looks like more errors come out on a s390x host for
>> some reason. You can run manually for a more complete test with more
>> coverage:
>>
>>   ./fp-test -l 2 -r all i32_to_f128
>>
>>>
>>>
>>>> diff --git a/tests/fp/Makefile b/tests/fp/Makefile
>>>> index 5019dcdca0..5a35e7c210 100644
>>>> --- a/tests/fp/Makefile
>>>> +++ b/tests/fp/Makefile
>>>> @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
>>>>  TF_CFLAGS :=
>>>>  TF_CFLAGS += -Wno-strict-prototypes
>>>>  TF_CFLAGS += -Wno-unknown-pragmas
>>>> -TF_CFLAGS += -Wno-discarded-qualifiers
>>>> -TF_CFLAGS += -Wno-maybe-uninitialized
>>>> +TF_CFLAGS += -Wno-uninitialized
>>>>  TF_CFLAGS += -Wno-missing-prototypes
>>>>  TF_CFLAGS += -Wno-return-type
>>>>  TF_CFLAGS += -Wno-unused-function
>>>
>>> configure has logic to check whether it can use particular
>>> warning enable/disable flags. Newer gcc (and I hope clang
>>> but forget) will happily silently allow -Wno-random-new-thing
>>> even if they don't support -Wrandom-new-thing) but I'm not
>>> sure our minimum compiler version is yet new enough to
>>> be able to rely on that (indeed the warning messages suggest
>>> it is not).
>>>
>>>> diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
>>>> index ca9fa2ba05..5a59dcec19 160000
>>>> --- a/tests/fp/berkeley-testfloat-3
>>>> +++ b/tests/fp/berkeley-testfloat-3
>>>> @@ -1 +1 @@
>>>> -Subproject commit ca9fa2ba05625ba929958f163b01747e07dd39cc
>>>> +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
>>>> diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
>>>> index fca576309c..2a35ef601d 100644
>>>> --- a/tests/fp/fp-test.c
>>>> +++ b/tests/fp/fp-test.c
>>>> @@ -789,7 +789,7 @@ static int set_init_flags(const char *flags)
>>>>      return 0;
>>>>  }
>>>>
>>>> -static uint8_t slow_clear_flags(void)
>>>> +static uint_fast8_t slow_clear_flags(void)
>>>>  {
>>>>      uint8_t prev = slowfloat_exceptionFlags;
>>>>
>>>> @@ -797,7 +797,7 @@ static uint8_t slow_clear_flags(void)
>>>>      return prev;
>>>>  }
>>>>
>>>> -static uint8_t qemu_clear_flags(void)
>>>> +static uint_fast8_t qemu_clear_flags(void)
>>>>  {
>>>>      uint8_t prev = qemu_flags_to_sf(qsf.float_exception_flags);
>>>
>>> Why are we using uint_fast8_t here anyway? We switched
>>> softfloat to using plain old uint8_t everywhere a while
>>> back.
>>>
>>> thanks
>>> -- PMM
>>
>>
>> --
>> Alex Bennée
>>


--
Alex Bennée

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Alex Bennée]

Emilio G. Cota <cota@braap.org> writes:

> On Thu, Jan 17, 2019 at 18:55:33 +0000, Peter Maydell wrote:
>> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
>> > What are the contents of "int-to-float.err"?
>>
>> linux1@lxub05:~$ cat qemu/build/all/tests/fp/int-to-float.err
>> >> Testing i32_to_f16, rounding near_even
>> 372 tests total.
>> 372 tests performed.
>> >> Testing i64_to_f16, rounding near_even
>> 756 tests total.
>> 756 tests performed.
>> >> Testing i32_to_f32, rounding near_even
>> 372 tests total.
>> 372 tests performed.
>> >> Testing i64_to_f32, rounding near_even
>> 756 tests total.
>> 756 tests performed.
>> >> Testing i32_to_f64
>> 372 tests total.
>> 372 tests performed.
>> >> Testing i64_to_f64, rounding near_even
>> 756 tests total.
>> 756 tests performed.
>> >> Testing i32_to_f128
>> 372 tests total.
>> 21 tests performed; 20 errors found.
>
> I see, so i32_to_f128 is failing on this host. Is there
> a s390x machine I could access? I don't see one in the
> gcc compile farm.

I've managed to reproduce this in a s390x VM, for Debian install runes:

  https://wiki.qemu.org/Documentation/Platforms/S390X#Debian_Install_Example_.28TCG.29

>
>> > diff --git a/tests/fp/Makefile b/tests/fp/Makefile
>> > index 5019dcdca0..5a35e7c210 100644
>> > --- a/tests/fp/Makefile
>> > +++ b/tests/fp/Makefile
>> > @@ -65,8 +65,7 @@ QEMU_CFLAGS += $(TF_OPTS)
>> >  TF_CFLAGS :=
>> >  TF_CFLAGS += -Wno-strict-prototypes
>> >  TF_CFLAGS += -Wno-unknown-pragmas
>> > -TF_CFLAGS += -Wno-discarded-qualifiers
>> > -TF_CFLAGS += -Wno-maybe-uninitialized
>> > +TF_CFLAGS += -Wno-uninitialized
>> >  TF_CFLAGS += -Wno-missing-prototypes
>> >  TF_CFLAGS += -Wno-return-type
>> >  TF_CFLAGS += -Wno-unused-function
>>
>> configure has logic to check whether it can use particular
>> warning enable/disable flags. Newer gcc (and I hope clang
>> but forget) will happily silently allow -Wno-random-new-thing
>> even if they don't support -Wrandom-new-thing) but I'm not
>> sure our minimum compiler version is yet new enough to
>> be able to rely on that (indeed the warning messages suggest
>> it is not).
>
> I figured that -Wno-unitialized is documented to work on both
> gcc and clang, and the change in testfloat drops the need
> for no-discarded-qualifiers, so we can go with this without
> messing with configure.

Works for me, OK with you Peter?

--
Alex Bennée

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Peter Maydell]

On Fri, 18 Jan 2019 at 17:41, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Emilio G. Cota <cota@braap.org> writes:
>
> > On Thu, Jan 17, 2019 at 18:55:33 +0000, Peter Maydell wrote:
> >> configure has logic to check whether it can use particular
> >> warning enable/disable flags. Newer gcc (and I hope clang
> >> but forget) will happily silently allow -Wno-random-new-thing
> >> even if they don't support -Wrandom-new-thing) but I'm not
> >> sure our minimum compiler version is yet new enough to
> >> be able to rely on that (indeed the warning messages suggest
> >> it is not).
> >
> > I figured that -Wno-unitialized is documented to work on both
> > gcc and clang, and the change in testfloat drops the need
> > for no-discarded-qualifiers, so we can go with this without
> > messing with configure.
>
> Works for me, OK with you Peter?

If it builds on all the build machines it's ok. If it doesn't
I'll bounce the pullreq and you can figure out a different
approach :-)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Emilio G. Cota]

On Fri, Jan 18, 2019 at 17:00:17 +0000, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> > Can you modify the Makefile.include to use:
> >  "fp-test ... 2>int-to-float.err || {cat int-to-float.err && exit 1;}"
> 
> Well it's in the test-softfloat macro but sure...

Actually, can we do &> instead of 2> ?

The errors coming from berkeley-testfloat are printed to stdout,
so we should get those too. (We could fix testfloat to print
to stderr, but I'd rather only make strictly necessary changes
to berkeley-testfloat.)

Thanks,

		E.

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Peter Maydell]

On Fri, 18 Jan 2019 at 18:16, Emilio G. Cota <cota@braap.org> wrote:
>
> On Fri, Jan 18, 2019 at 17:00:17 +0000, Alex Bennée wrote:
> > Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> > > Can you modify the Makefile.include to use:
> > >  "fp-test ... 2>int-to-float.err || {cat int-to-float.err && exit 1;}"
> >
> > Well it's in the test-softfloat macro but sure...
>
> Actually, can we do &> instead of 2> ?

"&>" is a bashism. The POSIX equivalent is >file.err 2>&1

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 18 Jan 2019 at 18:16, Emilio G. Cota <cota@braap.org> wrote:
>>
>> On Fri, Jan 18, 2019 at 17:00:17 +0000, Alex Bennée wrote:
>> > Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> > > Can you modify the Makefile.include to use:
>> > >  "fp-test ... 2>int-to-float.err || {cat int-to-float.err && exit 1;}"
>> >
>> > Well it's in the test-softfloat macro but sure...
>>
>> Actually, can we do &> instead of 2> ?
>
> "&>" is a bashism. The POSIX equivalent is >file.err 2>&1
>
> thanks
> -- PMM

I've gone for this:

 # $1 = tests, $2 = description
 test-softfloat = $(call quiet-command, \
-                        cd $(BUILD_DIR)/tests/fp && ./fp-test -s $(FP_TL) $1 > $2.out 2> $2.err, \
+                        cd $(BUILD_DIR)/tests/fp && \
+                        ./fp-test -s $(FP_TL) $1 > $2.out 2>&1 || \
+                        (cat $2.out && exit 1;), \
                         "FLOAT TEST", $2)

As for the tests it seems quite a lot are broken on s390x for a reason I
don't quite follow yet:

 i32_to_f128
 i64_to_f128
 ui64_to_f128
 extF80_to_i32
 extF80_to_i32_r_minMag
 extF80_eq

and more which I haven't confirmed yet. I'm tempted to ifndef the tests
for s390 for now while we sort out what's going on. Should I re-send
with that?

--
Alex Bennée

Re: [Qemu-devel] [PULL 0/7] check-softfloat, fp-bench and clang compile fixes

[Emilio G. Cota]

On Fri, Jan 18, 2019 at 17:41:15 +0000, Alex Bennée wrote:
> 
> Emilio G. Cota <cota@braap.org> writes:
> 
> > On Thu, Jan 17, 2019 at 18:55:33 +0000, Peter Maydell wrote:
> >> On Thu, 17 Jan 2019 at 18:30, Emilio G. Cota <cota@braap.org> wrote:
> >> > What are the contents of "int-to-float.err"?
> >>
> >> linux1@lxub05:~$ cat qemu/build/all/tests/fp/int-to-float.err
(snip)
> >> >> Testing i32_to_f128
> >> 372 tests total.
> >> 21 tests performed; 20 errors found.
> >
> > I see, so i32_to_f128 is failing on this host. Is there
> > a s390x machine I could access? I don't see one in the
> > gcc compile farm.
> 
> I've managed to reproduce this in a s390x VM, for Debian install runes:
> 
>   https://wiki.qemu.org/Documentation/Platforms/S390X#Debian_Install_Example_.28TCG.29

It's an endianness issue -- I've reproduced it on both gcc110
(POWER7) from gcc's compile farm and on the s390 machine provided
by Peter (thanks!). Both machines are big endian.

I have a fix for the 128 conversions (below), but not for the
int-to-f80 conversions (the ones that work on little endian).
I have no more time to look into this, so if someone can take
a look, I'd appreciate it.

I'm a little puzzled because given these definitions:

// our floatx80
typedef struct {
    uint64_t low;
    uint16_t high;
} floatx80;

// softfloat's
#ifdef LITTLEENDIAN
struct extFloat80M { uint64_t signif; uint16_t signExp; };
#else
struct extFloat80M { uint16_t signExp; uint64_t signif; };
#endif

I fail to see why just copying low/high to/from signif/signExp
to convert between them fails to work.

Thanks,

		Emilio

---
diff --git a/tests/fp/wrap.inc.c b/tests/fp/wrap.inc.c
index d3bf600cd0..68f57bc167 100644
--- a/tests/fp/wrap.inc.c
+++ b/tests/fp/wrap.inc.c
@@ -87,8 +87,13 @@ static float128_t qemu_to_soft128(float128 a)
     float128_t ret;
     struct uint128 *to = (struct uint128 *)&ret;

+#ifdef LITTLEENDIAN
     to->v0 = a.low;
     to->v64 = a.high;
+#else
+    to->v0 = a.high;
+    to->v64 = a.low;
+#endif
     return ret;
 }

@@ -97,8 +102,13 @@ static float128 soft_to_qemu128(float128_t a)
     struct uint128 *from = (struct uint128 *)&a;
     float128 ret;

+#ifdef LITTLEENDIAN
     ret.low = from->v0;
     ret.high = from->v64;
+#else
+    ret.low = from->v64;
+    ret.high = from->v0;
+#endif
     return ret;
 }