Skip to content

Comments

WIP Dynamic linking of BLAS/LAPACK in scipy#473

Closed
rth wants to merge 1 commit intopyodide:masterfrom
rth:scipy-dyn-link
Closed

WIP Dynamic linking of BLAS/LAPACK in scipy#473
rth wants to merge 1 commit intopyodide:masterfrom
rth:scipy-dyn-link

Conversation

@rth
Copy link
Member

@rth rth commented Jun 19, 2019

Work in progress on dynamic linking of BLAS/LAPACK in scipy, which should remove the scipy package size significantly. This could be later extended to numpy and BLAS.

Closes #240
Closes #386
Closes #416

@rth
Copy link
Member Author

rth commented Jun 21, 2019

I finally found time to look into this. So far, it is failing with exactly the same error previously in #211 (comment) while running validation in asm2wasm,

/src/emsdk/emsdk/binaryen/tag-1.38.30_64bit_binaryen/bin/asm2wasm build/lib.linux-x86_64-3.7/scipy/linalg/_fblas.cpython-37m-x86
_64-linux-gnu.temp.asm.js --total-memory=16777216 --trap-mode=clamp -O3 --mem-init=build/lib.linux-x86_64-3.7/scipy/linalg/_fblas.cpython-37m-x
86_64-linux-gnu.wasm.mem --table-max=-1 --mem-max=-1 -g -o build/lib.linux-x86_64-3.7/scipy/linalg/_fblas.cpython-37m-x86_64-linux-gnu.wast -S 
--mvp-features

with,

[wasm-validator error in function $_wsdot_] 4 != 3: set_local type must match function, on 
[none] (set_local $5
 [f32] (f32.demote/f64
  [f64] (call $_sdot_)
 )
)

some function signatures don't seem to match. I think it might be something about libf2c, will try to link that statically.

I'm not sure if RUNTIME_LINKED_LIBS actually supports relative path. @navytux could you be able to confirm? In any case, I checked that putting liblas_WA.wasm under the same folder as where emcc is running from doesn't help. I think this error is unrelated to dynamic linking. Will keep investigating.

@navytux
Copy link

navytux commented Jun 26, 2019

@rth first of all thanks for trying.

I'm not sure if RUNTIME_LINKED_LIBS actually supports relative path. @navytux could you be able to confirm?

If I recall correctly, the library paths specified in RUNTIME_LINKED_LIBS are then embedded into generated .wasm module (look for needed_dynlibs_entries in https://github.com/WebAssembly/tool-conventions/blob/master/DynamicLinking.md#the-dylink-section) and at runtime those paths are used as URLs passed to fetch:

https://github.com/emscripten-core/emscripten/blob//14948aa1a96c/src/support.js#L188
https://github.com/emscripten-core/emscripten/blob//14948aa1a96c/src/support.js#L105-L115

Judging from this I would say that relative paths for linked DSOs should be supported.

The error indeed could be not related to dynamic linking. I suggest to proceed as @kripken said in WebAssembly/binaryen#2177 (comment).

Kirill

@rth
Copy link
Member Author

rth commented Jun 27, 2019

Thanks for the confirmation @navytux !

Yes, that's the plan I opened #476 (and #480 as a first step).

@navytux
Copy link

navytux commented Jun 28, 2019

@rth, good luck there!

@joemarshall
Copy link
Contributor

Hey, I think when I was changing things for upstream in my fork I may have fixed the things that make dynamic linking fail here.The key is the patch to binaryen and emscripten which makes dynamic calls run through the function pointer cast emulation.

I also have patches to scipy, numpy and clapack that fix the function definition conflicts, as upstream needed it, but they wouldn't matter with dynamic linking with fpcast emulation turned on.

They're in my fork at https://github.com/joemarshall/pyodide

Relevant patches in https://github.com/joemarshall/pyodide/tree/master/emsdk/patches

@rth
Copy link
Member Author

rth commented Jan 1, 2021

That's certainly a good news ! Thanks @joemarshall . I think we should probably first try to update to upstream emscripten backend following your comments in #476 before addressing this.

@rth rth mentioned this pull request Feb 14, 2021
@rth rth closed this in #1236 Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split scipy.data into multiple chunks to fit in cache Scipy package size impacts loading time Dynamic linking of BLAS / LAPACK

3 participants