[PATCH RFC 0/2] Add BinaryEn

  • Done
  • quality assurance status badge
Details
3 participants
  • Andreas Enge
  • Thompson, David
  • Liliana Marie Prikler
Owner
unassigned
Submitted by
Liliana Marie Prikler
Severity
normal
L
L
Liliana Marie Prikler wrote on 17 Feb 2023 21:45
(address . guix-patches@gnu.org)
2cc4a1091e54c0fec082de4ce2789dfc42470e54.camel@gmail.com
Hi Guix,

this series gets us a little closer to having a "full" WebAssembly stack.
It packages binaryen, on top of which other compilers such as emscripten
or AssemblyScript (a sort of Typescript?) are built.

However, there is a grain of salt. It appears binaryen has some rather
esoteric use for -msse2 on i686: Rather than performance, it wants it
for precision. Needless to say, this would break compatibility with
older CPUs. I'm wondering if we should simply drop i686 (and similarly
32-bit ARM) from supported-systems or whether there's a more clever
hack to use here.

Cheers

Liliana Marie Prikler (2):
gnu: Add python-filecheck.
gnu: Add binaryen.

gnu/packages/check.scm | 25 +++++++++++++++++++++++
gnu/packages/web.scm | 45 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)


base-commit: 312f1f41d3f3f3e5d2c36ff46920c6dce1c21a17
--
2.39.1
L
L
Liliana Marie Prikler wrote on 17 Feb 2023 21:27
[PATCH RFC 1/2] gnu: Add python-filecheck.
(address . 61586@debbugs.gnu.org)
ca155e12553a1d104c4b8d14343c8d68a640a09f.camel@gmail.com
* gnu/packages/check.scm (python-filecheck): New variable.
---
gnu/packages/check.scm | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

Toggle diff (38 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 3d14cb7585..c4e83d41ef 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -2364,6 +2364,31 @@ (define-public python-lit
failures.")
(license license:ncsa)))
+(define-public python-filecheck
+ (package
+ (name "python-filecheck")
+ (version "0.0.23")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/mull-project/FileCheck.py")
+ (commit (string-append "v" version))))
+ (file-name (git-file-name name version))
+ (sha256
+ (base32
+ "1gipw7x54nr6raxr681igpxn8jli5mwaznnj40xxks0pa5kvirs7"))))
+ (build-system pyproject-build-system)
+ (native-inputs (list python-lit python-pytest python-pylint
+ python-invoke poetry))
+ (home-page "https://filecheck.readthedocs.io/")
+ (synopsis "Pattern matching file verifier")
+ (description
+ "This package provides a Python port of LLVM's FileCheck utility. It
+can be used to assert that certain strings (or regular expressions) are present
+or missing in a given file. A typical application is the post-processing of
+test logs.")
+ (license license:asl2.0)))
+
;;; This is marked as a bootstrap package because it propagates bootstrapped
;;; versions of jaraco-context and jaraco-functools.
(define-public python-pytest-enabler-bootstrap
--
2.39.1
L
L
Liliana Marie Prikler wrote on 17 Feb 2023 21:28
[PATCH RFC 2/2] gnu: Add binaryen.
(address . 61586@debbugs.gnu.org)
5be345282b038321a7f3c1f5fb2df8560ceb653c.camel@gmail.com
* gnu/packages/web.scm (binaryen): New variable.
---
gnu/packages/web.scm | 45 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)

Toggle diff (58 lines)
diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 7e49f798ea..0b509fa34c 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -1558,6 +1558,51 @@ (define-public wabt
other systems that want to manipulate WebAssembly files.")
(license license:asl2.0)))
+(define-public binaryen
+ (package
+ (name "binaryen")
+ (version "112")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/WebAssembly/binaryen")
+ (commit (string-append "version_" version))))
+ (file-name (git-file-name name version))
+ (sha256
+ (base32
+ "0970iz22yjxgi27d67kwmrx4zq7hig3i6b92vmlp4c4bd1bacny5"))
+ (modules '((guix build utils)))
+ (snippet #~(begin
+ (substitute* "CMakeLists.txt"
+ (("add_subdirectory\\(third_party\\)")
+ "find_package(GTest)"))
+ (substitute* "test/gtest/CMakeLists.txt"
+ (("include_directory\\(.*third_pary.*\\)") ""))
+ (delete-file-recursively "third_party")))))
+ (build-system cmake-build-system)
+ (arguments
+ (list #:out-of-source? #f ; for tests
+ #:configure-flags #~(list "-DBUILD_LLVM_DWARF=OFF")
+ #:phases
+ #~(modify-phases %standard-phases
+ (add-before 'check 'delete-failing-tests
+ (lambda _
+ ;; DWARF support relies on bundling LLVM, so don't
+ (for-each delete-file
+ (find-files "test/passes"
+ ".*dwarf.*\\.(bin\\.txt\|wasm)"))
+ (delete-file "test/unit/test_dwarf.py")))
+ (replace 'check
+ (lambda* (#:key tests? #:allow-other-keys)
+ (invoke "python" "check.py"))))))
+ (native-inputs (list googletest node-lts python-wrapper
+ python-lit python-filecheck))
+ (home-page "https://github.com/WebAssembly/binaryen")
+ (synopsis "WebAssembly compiler")
+ (description "Binaryen is a compiler and toolchain infrastructure library
+written in C++, with a single-header C API as well as a Javascript API.")
+ (license license:asl2.0)))
+
(define-public wasm3
(package
(name "wasm3")
--
2.39.1
A
A
Andreas Enge wrote on 11 Mar 2023 23:48
BinaryEn
(address . 61586@debbugs.gnu.org)
ZA0FW589iSuSS/H9@jurong
Hello Liliana,

how about you start by pushing the python-filecheck package?

Do you have a pointer to BinaryEn using -msse2 for precision?
I am not familiar with SSE2, but a quick look-up on Wikipedia only shows
(packed) double floating point operations and packed integer arithmetic.
All these should be feasible directly in C, although maybe more slowly.

In the CMakeLists.txt file there are the following lines:
if(NOT EMSCRIPTEN)
if(CMAKE_SYSTEM_PROCESSOR MATCHES "^i.86$")
# wasm doesn't allow for x87 floating point math
add_compile_flag("-msse2")
add_compile_flag("-mfpmath=sse")
elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "^armv[2-6]" AND NOT CMAKE_CXX_FLAGS MATCHES "-mfpu=")
add_compile_flag("-mfpu=vfpv3")
endif()
endif()

So the -msse2 flag will not be added on arm; the package is compiled
successfully on aarch64, and I see no reason why in principal it should
not also work on armhf. Maybe there is a more precise test for x86_64
that could be used instead of
if(CMAKE_SYSTEM_PROCESSOR MATCHES "^i.86$") ?
Building on armhf and i686 currently fails (is "blocked" in QA parlance):
due to python-aioredis failing on i686
and node, python-simplejson and python-numpy on armhf:

Andreas
L
L
Liliana Marie Prikler wrote on 12 Mar 2023 09:16
a895e3af8df2d5c77d3bc08b6aee8944e075f097.camel@gmail.com
Am Samstag, dem 11.03.2023 um 23:48 +0100 schrieb Andreas Enge:
Toggle quote (1 lines)
> Hello Liliana,
Hi Andreas, please don't forget to add me in CC.

Toggle quote (1 lines)
> how about you start by pushing the python-filecheck package?
I'd only do that if it has another user and for the time being I don't
see that. Don't worry, I still got more python packages in my backlog
:)

Toggle quote (6 lines)
> Do you have a pointer to BinaryEn using -msse2 for precision?
> I am not familiar with SSE2, but a quick look-up on Wikipedia only
> shows (packed) double floating point operations and packed integer
> arithmetic.
> All these should be feasible directly in C, although maybe more
> slowly.
Possible, but more slowly in C doesn't translate that nicely if you
don't want to code up your own float/double types and you really don't
want that.

The problem here is that expressions like:
double a, b, c;
c = sqrt(a * a, b * b);
can use 80 bit intermediaries on x87 chips, which they don't when using
SSE2 – hence the precision argument. You would have to redefine all
basic operations for your floating point (which would still be doable
in C++ due to operator overloading, but be a major pain in the butt to
do correctly and well-tested, hence the deference to SSE2, I believe).

Toggle quote (15 lines)
> In the CMakeLists.txt file there are the following lines:
>   if(NOT EMSCRIPTEN)
>     if(CMAKE_SYSTEM_PROCESSOR MATCHES "^i.86$")
>       # wasm doesn't allow for x87 floating point math
>       add_compile_flag("-msse2")
>       add_compile_flag("-mfpmath=sse")
>     elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "^armv[2-6]" AND NOT
> CMAKE_CXX_FLAGS MATCHES "-mfpu=")
>       add_compile_flag("-mfpu=vfpv3")
>     endif()
>   endif()
>
> So the -msse2 flag will not be added on arm; the package is compiled
> successfully on aarch64, and I see no reason why in principal it
> should not also work on armhf. 
It does require the vfpv3 fpu, which I believe won't exist on all arms.

Toggle quote (3 lines)
> Maybe there is a more precise test for x86_64 that could be used
> instead of
>     if(CMAKE_SYSTEM_PROCESSOR MATCHES "^i.86$") ?
Not for the kind of check they want to make, I believe.

Cheers
A
A
Andreas Enge wrote on 12 Mar 2023 11:49
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 61586@debbugs.gnu.org)
ZA2uSyEbKgBkEGFS@jurong
Hello,

Am Sun, Mar 12, 2023 at 09:16:45AM +0100 schrieb Liliana Marie Prikler:
Toggle quote (4 lines)
> Am Samstag, dem 11.03.2023 um 23:48 +0100 schrieb Andreas Enge:
> > Hello Liliana,
> Hi Andreas, please don't forget to add me in CC.

ah, I thought that debbugs would automatically redispatch the mail
to everybody who contributed to the report.

Toggle quote (9 lines)
> The problem here is that expressions like:
> double a, b, c;
> c = sqrt(a * a, b * b);
> can use 80 bit intermediaries on x87 chips, which they don't when using
> SSE2 – hence the precision argument. You would have to redefine all
> basic operations for your floating point (which would still be doable
> in C++ due to operator overloading, but be a major pain in the butt to
> do correctly and well-tested, hence the deference to SSE2, I believe).

Hm, I thought that when using gcc and glibc, floating point operations
now followed the IEEE-754 standard. Then it would not matter what the
internal format is (except for special functions).

Still, I wonder if it would not be possible to change the configure test
so that the package compiles on 32 bit platforms; otherwise we would have
to take them out from the supported systems. And that would not be better
(assuming that the package passes its tests on these platforms, of course).

Andreas
T
T
Thompson, David wrote on 6 Apr 2023 23:38
Re: [bug#61586] [PATCH RFC 2/2] gnu: Add binaryen.
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 61586@debbugs.gnu.org)
CAJ=RwfbuBdcn3yQ7U24ghSNiH95H+jt_BZse+P0mjmp5GRj2=Q@mail.gmail.com
Hi Liliana,

On Fri, Feb 17, 2023 at 6:00?PM Liliana Marie Prikler
<liliana.prikler@gmail.com> wrote:
Toggle quote (65 lines)
>
> * gnu/packages/web.scm (binaryen): New variable.
> ---
> gnu/packages/web.scm | 45 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 7e49f798ea..0b509fa34c 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -1558,6 +1558,51 @@ (define-public wabt
> other systems that want to manipulate WebAssembly files.")
> (license license:asl2.0)))
>
> +(define-public binaryen
> + (package
> + (name "binaryen")
> + (version "112")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/WebAssembly/binaryen")
> + (commit (string-append "version_" version))))
> + (file-name (git-file-name name version))
> + (sha256
> + (base32
> + "0970iz22yjxgi27d67kwmrx4zq7hig3i6b92vmlp4c4bd1bacny5"))
> + (modules '((guix build utils)))
> + (snippet #~(begin
> + (substitute* "CMakeLists.txt"
> + (("add_subdirectory\\(third_party\\)")
> + "find_package(GTest)"))
> + (substitute* "test/gtest/CMakeLists.txt"
> + (("include_directory\\(.*third_pary.*\\)") ""))
> + (delete-file-recursively "third_party")))))
> + (build-system cmake-build-system)
> + (arguments
> + (list #:out-of-source? #f ; for tests
> + #:configure-flags #~(list "-DBUILD_LLVM_DWARF=OFF")
> + #:phases
> + #~(modify-phases %standard-phases
> + (add-before 'check 'delete-failing-tests
> + (lambda _
> + ;; DWARF support relies on bundling LLVM, so don't
> + (for-each delete-file
> + (find-files "test/passes"
> + ".*dwarf.*\\.(bin\\.txt\|wasm)"))
> + (delete-file "test/unit/test_dwarf.py")))
> + (replace 'check
> + (lambda* (#:key tests? #:allow-other-keys)
> + (invoke "python" "check.py"))))))
> + (native-inputs (list googletest node-lts python-wrapper
> + python-lit python-filecheck))
> + (home-page "https://github.com/WebAssembly/binaryen")
> + (synopsis "WebAssembly compiler")
> + (description "Binaryen is a compiler and toolchain infrastructure library
> +written in C++, with a single-header C API as well as a Javascript API.")
> + (license license:asl2.0)))
> +
> (define-public wasm3
> (package
> (name "wasm3")
> --
> 2.39.1

This looks good to me! We are currently using a hackier package
recipe with tests disabled in the Guile Hoot (Guile -> WASM compiler)

Nice job getting the tests working!

- Dave
L
L
Liliana Marie Prikler wrote on 11 Jun 2023 10:04
(name . Thompson, David)(address . dthompson2@worcester.edu)(address . 61586-done@debbugs.gnu.org)
a2377ddc9b7fa74dc4825cb718210f1ff3a28bbe.camel@gmail.com
Am Donnerstag, dem 06.04.2023 um 17:38 -0400 schrieb Thompson, David:
Toggle quote (9 lines)
> Hi Liliana,
>
> [...]
> This looks good to me!  We are currently using a hackier package
> recipe with tests disabled in the Guile Hoot (Guile -> WASM compiler)
> project:
> https://gitlab.com/spritely/guile-hoot-updates/-/blob/main/examples/manifest.scm#L18
>
> Nice job getting the tests working!
Someone (I don't remember who and don't care to look it up, might have
been myself) pushed this without marking the bug as done. The hoot
link is also dead. Time to close up.

Cheers
Closed
?