[Qemu-devel] [PATCH v2 0/7] current fpu/next queue

[Qemu-devel] [PATCH v2 0/7] current fpu/next queue

[Alex Bennée]

Hi,

I plan to send a pull request for this tomorrow. Two changes were:

 - take Emilio's v2 of the FMA patch
 - add my version of the s390x+clang compile fix

Unless there is a strong performance argument I'm minded to go with my
version. The numbers so far look pretty marginal for all 3 code paths.

It would be nice to get some review on the last few patches:

The following patches need review:
 patch 0004/softfloat fallback to __int128 maths for s390x an.patch
 patch 0005/tests Makefile add floating point tests.patch
 patch 0007/tests Makfile add check softfloat rule.patch

Alex Bennée (4):
  softfloat: fallback to __int128 maths for s390x and others
  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

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

-- 
2.17.1

[Qemu-devel] [PATCH v2 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>
---
 tests/fp/fp-bench.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

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] [PATCH v2 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>
---
 tests/fp/fp-bench.c | 4 ----
 1 file changed, 4 deletions(-)

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] [PATCH v2 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>
---
 fpu/softfloat.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

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] [PATCH v2 4/7] softfloat: fallback to __int128 maths for s390x and others

[Alex Bennée]

Apparently some versions of clang can't handle inline assembly with
__int128 parameters, especially on s390. Instead of hand-coding the
s390 divide provide a generic fallback for anything that provides
__int128 capable maths.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Thomas Huth <thuth@redhat.com>
---
 include/fpu/softfloat-macros.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index b1d772e6d4..1a43609eef 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -641,12 +641,6 @@ 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__)
-    /* 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));
-    *r = n >> 64;
-    return n;
 #elif defined(_ARCH_PPC64) && defined(_ARCH_PWR7)
     /* From Power ISA 2.06, programming note for divdeu.  */
     uint64_t q1, q2, Q, r1, r2, R;
@@ -663,6 +657,10 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
     }
     *r = R;
     return Q;
+#elif defined(CONFIG_INT128)
+    unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
+    *r = n % d;
+    return n / d;
 #else
     uint64_t d0, d1, q0, q1, r1, r0, m;
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 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>
---
 tests/Makefile.include | 115 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

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] [PATCH v2 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>
---
 scripts/archive-source.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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] [PATCH v2 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>
---
 tests/Makefile.include | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

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] [PATCH v2 3/7] softfloat: enforce softfloat if the host's FMA is broken

[Richard Henderson]

On 1/17/19 7:23 AM, Alex Bennée wrote:
> 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>
> ---
>  fpu/softfloat.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v2 4/7] softfloat: fallback to __int128 maths for s390x and others

[Richard Henderson]

On 1/17/19 7:23 AM, Alex Bennée wrote:
> Apparently some versions of clang can't handle inline assembly with
> __int128 parameters, especially on s390. Instead of hand-coding the
> s390 divide provide a generic fallback for anything that provides
> __int128 capable maths.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> ---
>  include/fpu/softfloat-macros.h | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
> index b1d772e6d4..1a43609eef 100644
> --- a/include/fpu/softfloat-macros.h
> +++ b/include/fpu/softfloat-macros.h
> @@ -641,12 +641,6 @@ 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__)
> -    /* 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));
> -    *r = n >> 64;
> -    return n;
>  #elif defined(_ARCH_PPC64) && defined(_ARCH_PWR7)
>      /* From Power ISA 2.06, programming note for divdeu.  */
>      uint64_t q1, q2, Q, r1, r2, R;
> @@ -663,6 +657,10 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
>      }
>      *r = R;
>      return Q;
> +#elif defined(CONFIG_INT128)
> +    unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
> +    *r = n % d;
> +    return n / d;
>  #else

I thought that we'd shown that, at least at present, no compiler is taking
advantage of hardware insns for this, and is promoting this to a full 128-bit
divide.  And further that the version using 64-bit arithmetic was competitive
with the hardware insn.

I'd rather not include this hunk for now.


r~

Re: [Qemu-devel] [PATCH v2 7/7] tests/Makfile: add check-softfloat rule

[Richard Henderson]

On 1/17/19 7:23 AM, Alex Bennée wrote:
> 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>
> ---
>  tests/Makefile.include | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v2 4/7] softfloat: fallback to __int128 maths for s390x and others

[Thomas Huth]

On 2019-01-16 21:23, Alex Bennée wrote:
> Apparently some versions of clang can't handle inline assembly with
> __int128 parameters, especially on s390. Instead of hand-coding the
> s390 divide provide a generic fallback for anything that provides
> __int128 capable maths.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> ---
>  include/fpu/softfloat-macros.h | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
> index b1d772e6d4..1a43609eef 100644
> --- a/include/fpu/softfloat-macros.h
> +++ b/include/fpu/softfloat-macros.h
> @@ -641,12 +641,6 @@ 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__)
> -    /* 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));
> -    *r = n >> 64;
> -    return n;
>  #elif defined(_ARCH_PPC64) && defined(_ARCH_PWR7)
>      /* From Power ISA 2.06, programming note for divdeu.  */
>      uint64_t q1, q2, Q, r1, r2, R;
> @@ -663,6 +657,10 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
>      }
>      *r = R;
>      return Q;
> +#elif defined(CONFIG_INT128)
> +    unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
> +    *r = n % d;
> +    return n / d;
>  #else
>      uint64_t d0, d1, q0, q1, r1, r0, m;

No, please don't. Use my !defined(__clang__) patch instead, please.

 Thomas

Re: [Qemu-devel] [PATCH v2 4/7] softfloat: fallback to __int128 maths for s390x and others

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/17/19 7:23 AM, Alex Bennée wrote:
>> Apparently some versions of clang can't handle inline assembly with
>> __int128 parameters, especially on s390. Instead of hand-coding the
>> s390 divide provide a generic fallback for anything that provides
>> __int128 capable maths.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Thomas Huth <thuth@redhat.com>
>> ---
>>  include/fpu/softfloat-macros.h | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
>> index b1d772e6d4..1a43609eef 100644
>> --- a/include/fpu/softfloat-macros.h
>> +++ b/include/fpu/softfloat-macros.h
>> @@ -641,12 +641,6 @@ 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__)
>> -    /* 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));
>> -    *r = n >> 64;
>> -    return n;
>>  #elif defined(_ARCH_PPC64) && defined(_ARCH_PWR7)
>>      /* From Power ISA 2.06, programming note for divdeu.  */
>>      uint64_t q1, q2, Q, r1, r2, R;
>> @@ -663,6 +657,10 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
>>      }
>>      *r = R;
>>      return Q;
>> +#elif defined(CONFIG_INT128)
>> +    unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
>> +    *r = n % d;
>> +    return n / d;
>>  #else
>
> I thought that we'd shown that, at least at present, no compiler is taking
> advantage of hardware insns for this, and is promoting this to a full 128-bit
> divide.  And further that the version using 64-bit arithmetic was competitive
> with the hardware insn.

Yeah it seems so. While Thomas' numbers weren't convincing the
CONFIG_INT128 fallback did trigger on my SynQuacer an knocked off about
2 MFlops of it's admittedly slow performance. Amusingly of course it's
faster under translation because of the hardware fall back:

07:44:44 [alex@idun:~/l/q/t/fp] (8973c1e5…) + ./fp-bench -o div -p double
13.28 MFlops
07:44:49 [alex@idun:~/l/q/t/fp] (8973c1e5…) + ./fp-bench -o div -p double -t host
498.20 MFlops
07:44:53 [alex@idun:~/l/q/t/fp] (8973c1e5…) + ../../aarch64-linux-user/qemu-aarch64  ./fp-bench -o div -p double -t host
52.71 MFlops

I'll drop this and use Thomas' #elif defined(__s390x__) &&
!defined(__clang__) version in the pull-request.

--
Alex Bennée