[01/23] lib: move muldiv64()

Commit Message

Jan Beulich April 1, 2021, 10:19 a.m. UTC

Make this a separate archive member under lib/. While doing so, don't
move latently broken x86 assembly though: Fix the constraints, such
that properly extending inputs to 64-bit won't just be a side effect of
needing to copy registers, and such that we won't fail to clobber %rdx.

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Jan Beulich April 15, 2021, 10 a.m. UTC | #1

On 01.04.2021 12:19, Jan Beulich wrote:
> Make this a separate archive member under lib/. While doing so, don't
> move latently broken x86 assembly though: Fix the constraints, such
> that properly extending inputs to 64-bit won't just be a side effect of
> needing to copy registers, and such that we won't fail to clobber %rdx.
> 
> Signed-off-by: Jan Beulich <[e-mail]>

While this is a change to common code, there being a specific x86
aspect here I can understand that Julien would prefer to not ack
this without an x86 maintainer having reviewed the change. May I
ask for feedback here, as I'd prefer to commit at least the first
few patches of this series before re-posting the rest.

Thanks, Jan

> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -393,35 +393,6 @@ s64 __ldivmod_helper(s64 a, s64 b, s64 *
>  }
>  #endif /* BITS_PER_LONG == 32 */
>  
> -/* Compute with 96 bit intermediate result: (a*b)/c */
> -uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> -{
> -#ifdef CONFIG_X86
> -    asm ( "mul %%rdx; div %%rcx" : "=a" (a) : "0" (a), "d" (b), "c" (c) );
> -    return a;
> -#else
> -    union {
> -        uint64_t ll;
> -        struct {
> -#ifdef WORDS_BIGENDIAN
> -            uint32_t high, low;
> -#else
> -            uint32_t low, high;
> -#endif            
> -        } l;
> -    } u, res;
> -    uint64_t rl, rh;
> -
> -    u.ll = a;
> -    rl = (uint64_t)u.l.low * (uint64_t)b;
> -    rh = (uint64_t)u.l.high * (uint64_t)b;
> -    rh += (rl >> 32);
> -    res.l.high = rh / c;
> -    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
> -    return res.ll;
> -#endif
> -}
> -
>  /*
>   * Local variables:
>   * mode: C
> --- a/xen/lib/Makefile
> +++ b/xen/lib/Makefile
> @@ -4,6 +4,7 @@ lib-y += bsearch.o
>  lib-y += ctors.o
>  lib-y += ctype.o
>  lib-y += list-sort.o
> +lib-y += muldiv64.o
>  lib-y += parse-size.o
>  lib-y += rbtree.o
>  lib-y += sort.o
> --- /dev/null
> +++ b/xen/lib/muldiv64.c
> @@ -0,0 +1,44 @@
> +#include <xen/lib.h>
> +
> +/* Compute with 96 bit intermediate result: (a*b)/c */
> +uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> +{
> +#ifdef CONFIG_X86
> +    asm ( "mulq %1; divq %2" : "+a" (a)
> +                             : "rm" ((uint64_t)b), "rm" ((uint64_t)c)
> +                             : "rdx" );
> +
> +    return a;
> +#else
> +    union {
> +        uint64_t ll;
> +        struct {
> +#ifdef WORDS_BIGENDIAN
> +            uint32_t high, low;
> +#else
> +            uint32_t low, high;
> +#endif
> +        } l;
> +    } u, res;
> +    uint64_t rl, rh;
> +
> +    u.ll = a;
> +    rl = (uint64_t)u.l.low * (uint64_t)b;
> +    rh = (uint64_t)u.l.high * (uint64_t)b;
> +    rh += (rl >> 32);
> +    res.l.high = rh / c;
> +    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
> +
> +    return res.ll;
> +#endif
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 
>

Roger Pau Monne April 15, 2021, 12:46 p.m. UTC | #2

On Thu, Apr 01, 2021 at 12:19:16PM +0200, Jan Beulich wrote:
> Make this a separate archive member under lib/. While doing so, don't
> move latently broken x86 assembly though: Fix the constraints, such
> that properly extending inputs to 64-bit won't just be a side effect of
> needing to copy registers, and such that we won't fail to clobber %rdx.
> 
> Signed-off-by: Jan Beulich <[e-mail]>

The x86 part LGTM:

Acked-by: Roger Pau Monné <[e-mail]>

Thanks, Roger.

Patch

--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -393,35 +393,6 @@  s64 __ldivmod_helper(s64 a, s64 b, s64 *
 }
 #endif /* BITS_PER_LONG == 32 */
 
-/* Compute with 96 bit intermediate result: (a*b)/c */
-uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
-{
-#ifdef CONFIG_X86
-    asm ( "mul %%rdx; div %%rcx" : "=a" (a) : "0" (a), "d" (b), "c" (c) );
-    return a;
-#else
-    union {
-        uint64_t ll;
-        struct {
-#ifdef WORDS_BIGENDIAN
-            uint32_t high, low;
-#else
-            uint32_t low, high;
-#endif            
-        } l;
-    } u, res;
-    uint64_t rl, rh;
-
-    u.ll = a;
-    rl = (uint64_t)u.l.low * (uint64_t)b;
-    rh = (uint64_t)u.l.high * (uint64_t)b;
-    rh += (rl >> 32);
-    res.l.high = rh / c;
-    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
-    return res.ll;
-#endif
-}
-
 /*
  * Local variables:
  * mode: C
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,6 +4,7 @@  lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += muldiv64.o
 lib-y += parse-size.o
 lib-y += rbtree.o
 lib-y += sort.o
--- /dev/null
+++ b/xen/lib/muldiv64.c
@@ -0,0 +1,44 @@ 
+#include <xen/lib.h>
+
+/* Compute with 96 bit intermediate result: (a*b)/c */
+uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+{
+#ifdef CONFIG_X86
+    asm ( "mulq %1; divq %2" : "+a" (a)
+                             : "rm" ((uint64_t)b), "rm" ((uint64_t)c)
+                             : "rdx" );
+
+    return a;
+#else
+    union {
+        uint64_t ll;
+        struct {
+#ifdef WORDS_BIGENDIAN
+            uint32_t high, low;
+#else
+            uint32_t low, high;
+#endif
+        } l;
+    } u, res;
+    uint64_t rl, rh;
+
+    u.ll = a;
+    rl = (uint64_t)u.l.low * (uint64_t)b;
+    rh = (uint64_t)u.l.high * (uint64_t)b;
+    rh += (rl >> 32);
+    res.l.high = rh / c;
+    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
+
+    return res.ll;
+#endif
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */