Skip to content
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

Improvement/field inv pornin20 #106

Merged
merged 71 commits into from
Dec 9, 2021
Merged

Conversation

Tabaie
Copy link
Contributor

@Tabaie Tabaie commented Dec 6, 2021

Benchmarked on a VM with an AMD EPYC 7R32 processor

For bw6-761/fp speedup ranged from 71% to 77%.
For bn254/fp speedup ranged from 43% to 65%.

Speedup on my ARM laptop was nowhere near as dramatic.

The inconsistency in the small field performance is due to higher variability in the number of outer loop iterations, a random effect that gets smoothed out in higher field size. In general, the algorithm has a good asymptotic complexity and is expected to scale well.

TODOs:

  1. Remove InverseOld and mulWRegularBf (cleanup)
  2. Implement the "two update factors in one register trick" described in the paper (perf)
  3. Implement some core functions in assembly (linearCombNonModular, mulWRegular, montReduceSigned) (perf)

Tabaie and others added 30 commits October 28, 2021 12:42
@Tabaie Tabaie requested a review from gbotrel December 6, 2021 08:44
@CLAassistant
Copy link

CLAassistant commented Dec 6, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ Tabaie
✅ gbotrel
❌ Arya Tabaie


Arya Tabaie seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

return b
}

// Though we're defining k as a constant, this code "profoundly" assumes that the processor is 64 bit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"profoundly" assumes --> is optimized ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't make a whole lot of sense. Removing.

// approximate a big number using its uppermost and lowermost bits
func approximate(x *{{.ElementName}}, n int) uint64 {

if n <= 64 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question, just based on the function comment; if say, n == 24, shouldn't this clear the 40 other bits to 0?

@gbotrel gbotrel merged commit c772392 into develop Dec 9, 2021
@gbotrel gbotrel deleted the improvement/field-inv-pornin20 branch December 9, 2021 18:44
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.

3 participants