-
Notifications
You must be signed in to change notification settings - Fork 413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds api.MAC(..)
#427
Conversation
…ency of debug info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I think it is a clean and required change.
func (builder *scs) MAC(a, b, c frontend.Variable) frontend.Variable { | ||
// TODO can we do better here to limit allocations? | ||
// technically we could do that in one PlonK constraint (against 2 for separate Add & Mul) | ||
return builder.Add(a, builder.Mul(b, c)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be done. And this is also helpful for the case if we want to have a transpiler from some other circuit description to gnark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am going to wait on @ThomasPiellard new backend on that one; plonk frontend is in a need of a refactor to convert input to term, and add custom constraints etc... keeping the TODO around for now.
Suggested edit: diff --git a/std/math/emulated/wrapped_api.go b/std/math/emulated/wrapped_api.go
index aa433d24..6d40f4ee 100644
--- a/std/math/emulated/wrapped_api.go
+++ b/std/math/emulated/wrapped_api.go
@@ -77,8 +77,10 @@ func (w *FieldAPI[T]) Add(i1 frontend.Variable, i2 frontend.Variable, in ...fron
}
func (w *FieldAPI[T]) MulAcc(a, b, c frontend.Variable) frontend.Variable {
- // TODO can we do better here to limit allocations?
- return w.Add(a, w.Mul(b, c))
+ els := w.varsToElements(a, b, c)
+ mul := w.f.Mul(els[1], els[2])
+ res := w.f.Add(els[0], mul)
+ return res
}
func (w *FieldAPI[T]) Neg(i1 frontend.Variable) frontend.Variable {
|
Tried to see if could reduce allocating new |
Fixes #416 . Impact in
std/math/emulated
forrsh
is significant (~40% less memallocs).Also, this new api would result in less constraint in a PlonKish arithmetization, since it will create one constraint instead of 2.