Permit code to be compiled as a plugin with Go 1.17.x #112
Conversation
|
|
|
See hyperledger/fabric#3073 for the point where the build is broken |
| // reduce element(R14,R15,CX,BX) using temp registers (R13,SI,R12,R11) | ||
| REDUCE(R14,R15,CX,BX,R13,SI,R12,R11) | ||
| // reduce element(R14,R12,CX,BX) using temp registers (R13,SI,R13,R11) | ||
| REDUCE(R14,R12,CX,BX,R13,SI,AX,R11) |
There was a problem hiding this comment.
I followed up to here.
R15 becomes R12.
R12 becomes R13.
But why AX here?
There was a problem hiding this comment.
It's indeed a typo - AX is used as a temp register by REDUCE. Will fix - thx Dave
In Go version 1.17.x, `R15` is reserved for the GOT (see golang/go#48468) when the code is built as a plugin, and code that uses `R15` will fail to build. This PR changes the designation of registers to fix the compilation errors. Signed-off-by: Alessandro Sorniotti <aso@zurich.ibm.com>
|
Interesting; it seems this happens when user assembly uses R15 and global variables in the same code. While your patch works for BN254 (thanks for the effort :-) ), it won't generalize well to the other curves supported in gnark-crypto, since they use all available registers. Found this PR that deals with the issue (and keep using R15) by moving global variables in defines: https://go-review.googlesource.com/c/go/+/36414/ . Will have a look and experiment with our x86 generator to make that work 👍 . |
|
( also, unrelated, I was not aware gnark-crypto was a direct dependency of hyperledger/fabric, to what extend is it used? ) |
|
The approach detailed in that link sounds promising - though it might introduce a perf penalty. Definitely worth investigating though. Fabric depends on gnark-crypto since it (fabric) pulls https://github.com/IBM/idemix/ which depends on https://github.com/IBM/mathlib/ which in turn uses gnark-crypto for one of the curves. In that sense, my current fix would be an acceptable stop-gap solution till you find one that generalises to all curves/functions. Obviously up to you/the maintainers of gnark-crypto to decide whether it's okay to stage changes this way. |
|
Many thanks, that was quick! |
In Go version 1.17.x,
R15is reserved for the GOT (seegolang/go#48468) when the code is built as a
plugin, and code that uses
R15will fail to build. This PR changesthe designation of registers to fix the compilation errors.
Signed-off-by: Alessandro Sorniotti aso@zurich.ibm.com