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

crypto: use purego tag consistently #58636

Closed
FiloSottile opened this issue Feb 22, 2023 · 7 comments
Closed

crypto: use purego tag consistently #58636

FiloSottile opened this issue Feb 22, 2023 · 7 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Feb 22, 2023

We've been inconsistent in using the purego tag in the standard library because it was not clear what would be its use aside from letting me benchmark assembly vs native code. I noticed on the Gophers Slack that tagging purego consistently would probably make TinyGo's life easier when trying to compile crypto packages, and presumably any other Go compiler. https://tinygo.org/docs/reference/lang-support/stdlib/#crypto

It's easy enough to do, so why not. Might also add a purego compilation test of crypto/... to longtest.

/cc @dgryski

@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 22, 2023
@FiloSottile FiloSottile added this to the Go1.21 milestone Feb 22, 2023
@FiloSottile FiloSottile self-assigned this Feb 22, 2023
@dgryski
Copy link
Contributor

dgryski commented Feb 22, 2023

The hash/crc32 package also needs purego tags.

@cuonglm
Copy link
Member

cuonglm commented Feb 22, 2023

FYI, the purego proposal is accepted #23172

I also had https://go-review.googlesource.com/c/go/+/468795, which implements a purego version of hash/maphash. There, you can add the test with purego tag in cmd/dist.

@gopherbot gopherbot modified the milestones: Go1.21, Go1.22 Aug 8, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/561935 mentions this issue: crypto: use and test purego tag consistently

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/561955 mentions this issue: TargetSpecific: update purego tag guidance

gopherbot pushed a commit to golang/wiki that referenced this issue Feb 6, 2024
Updates golang/go#58636

Change-Id: I28cbe38d750ae10f3f78af909de33424ff2a6241
Reviewed-on: https://go-review.googlesource.com/c/wiki/+/561955
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Commit-Queue: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@gopherbot gopherbot modified the milestones: Go1.22, Go1.23 Feb 6, 2024
@aykevl
Copy link

aykevl commented Feb 7, 2024

To be clear, purego means "use Go instead of assembly"?
I'm asking, because it could also be interpreted as "don't use CGo". In TinyGo we do support CGo (with some limitations) but Go assembly is very difficult due to the special syntax and different calling convention. So basically we'd like to set the purego tag by default to avoid assembly but would still like to use CGo.

In #23172 it seems that purego is defined as "no assembly, unsafe, or CGo", which combines a number of unrelated features and would not fit very well with TinyGo.

It's probably much too late to change, but noasm would be a much better fit for TinyGo because it's more explicit (and similarly, nounsafe could be used - for CGo we already have the cgo build tag).

Related: tinygo-org/tinygo#4116

@FiloSottile
Copy link
Contributor Author

To be clear, purego means "use Go instead of assembly"? I'm asking, because it could also be interpreted as "don't use CGo". In TinyGo we do support CGo (with some limitations) but Go assembly is very difficult due to the special syntax and different calling convention. So basically we'd like to set the purego tag by default to avoid assembly but would still like to use CGo.

In #23172 it seems that purego is defined as "no assembly, unsafe, or CGo", which combines a number of unrelated features and would not fit very well with TinyGo.

Indeed, it might be a bit coarse, but I think purego means "no assembly, unsafe, or cgo when there is a pure-Go alternative". There's no point making a build tag for breaking a build. So in practice it mostly means "no assembly" and only occasionally "no unsafe".

@aykevl
Copy link

aykevl commented Mar 15, 2024

Hmm, that's unfortunate. I imagine it could limit features/performance in some cases now that we use purego by default.

For CGo, I'd normally assume the cgo build tag would be used. That's the build tag specifically meant for that purpose as far as I know. While TinyGo doesn't support all CGo features, it's pretty close and missing parts are generally not too difficult to add.
TinyGo does support unsafe just fine though. It has a different memory layout for some types, but portable code would try to avoid relying on a specific memory layout anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants