Conversation
|
@SwayamInSync Maybe it would be even better to upstream the SLEEF patches for Pyodide? That way, the Pyodide recipe would be patch-free (something that Pyodide is working towards as it will transition from a centralised distribution towards package authors building Pyodide wheels themselves), and this repo could have full control over the patches |
Means incorporate in quaddtype's build? |
Yes, exactly! I already copied over your patch (now in the 0003-... file), so something similar but so that the patches are only applied when building for Pyodide (if that's possible)? |
|
In principle it is possible, but will require good work, as similar dispatching mechanism as in #62 and modifying all the patches to include a conditional flag that turns those patches ON/OFF. Also I am not sure, but there might be an ordering to apply patches. But these all can be figured out. Once get sometime to look through the details. |
| diff --git a/src/libm/dispavx.c.org b/src/libm/dispavx.c.org | ||
| index 86a0148..e309af7 100644 | ||
| --- a/src/libm/dispavx.c.org | ||
| +++ b/src/libm/dispavx.c.org |
There was a problem hiding this comment.
Interesting, I wonder 0003 patch and SLEEF's own autodetection should be enough to not dispatch these SIMD functions
There was a problem hiding this comment.
Oh maybe its because of cross-compilation, autodetection might fail.
There was a problem hiding this comment.
Auto detection doesn't exist at all in Emscripten (the instruction doesn't exist), so we need to fully patch them out, unfortunately
There was a problem hiding this comment.
Autodetection is in SLEEF, it detects the properties by compiling code via the assigned compiler (very similar to what we do)
There was a problem hiding this comment.
Oh, we were talking about different detections then (compile time vs runtime). You can try it again without the patch - from what I remember, there was a link error with sleef's runtime detection, so some dispatch code must still be linked in
|
Lets keep this, I'll see if I can update it post v1 release |
SwayamInSync
changed the title
That sounds like a good plan to me! |
|
@SwayamInSync Do we need anything else for this PR? |
|
I think this is good, if anything is needed I'll take care of it later |
agriyakhetarpal
left a comment
agriyakhetarpal
left a comment
There was a problem hiding this comment.
Thanks, @juntyr!
FYI, you can use https://github.com/pyodide/pyodide-build-environment-nightly/releases/tag/20260120 for the xbuildenv as it comes with numpy==2.2.6, which should avoid needing to add a recipe for NumPy, and hopefully using cibuildwheel will become a bit easier.
| python-version: 3.13.2 | ||
|
|
||
| - name: Setup Emscripten | ||
| uses: mymindstorm/setup-emsdk@v14 |
There was a problem hiding this comment.
| uses: mymindstorm/setup-emsdk@v14 | |
| uses: pyodide/setup-emsdk@v15 |
This action is not actively maintained and is waiting on mymindstorm/setup-emsdk#48. In the meantime, you can use our fork to save 2-3 minutes per run.
| - name: Install the Pyodide build env | ||
| run: pyodide xbuildenv install 0.29.2 |
There was a problem hiding this comment.
Why not use the cibuildwheel action with platform: pyodide (cibuildwheel --platform pyodide if using the CLI)?
|
@agriyakhetarpal we need numpy >=2.4, which Pyodide doesn't support yet (I think it's blocked on the scipy upgrade) |
3 participants
This adds recipes and CI to check that numpy-quaddtype works in Pyodide.
Pyodide currently uses an old version of numpy, and building libsleef requires some Pyodide-specific patches (we could upstream them into meson in the future), so we cannot use the simpler https://github.com/numpy/numpy/blob/main/.github/workflows/emscripten.yml yet, so instead, this is my own home-cooked, more verbose one.