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

all: add GFNI instructions #344

Merged
merged 10 commits into from
Nov 28, 2022
Merged

all: add GFNI instructions #344

merged 10 commits into from
Nov 28, 2022

Conversation

mmcloughlin
Copy link
Owner

Based on #343

Updates #335

@mmcloughlin mmcloughlin mentioned this pull request Nov 27, 2022
@mmcloughlin
Copy link
Owner Author

@klauspost It appears the opcodes without V prefix are not recognized by Go assembler:

--- FAIL: TestAssembles (1.24s)
    load_test.go:30: exec: [/opt/hostedtoolcache/go/1.18.8/x64/bin/go tool asm -e -o /tmp/avo2003297476/asm.o /tmp/avo2003297476/asm.s]
    load_test.go:30: output:
        /tmp/avo200329[74](https://github.com/mmcloughlin/avo/actions/runs/3559848371/jobs/5979483745#step:8:75)76/asm.s:841: unrecognized instruction "GF2P8AFFINEINVQB"
        /tmp/avo2003297476/asm.s:842: unrecognized instruction "GF2P8AFFINEINVQB"
        /tmp/avo20032974[76](https://github.com/mmcloughlin/avo/actions/runs/3559848371/jobs/5979483745#step:8:77)/asm.s:845: unrecognized instruction "GF2P8AFFINEQB"
        /tmp/avo2003297476/asm.s:846: unrecognized instruction "GF2P8AFFINEQB"
        /tmp/avo2003297476/asm.s:849: unrecognized instruction "GF2P8MULB"
        /tmp/avo2003297476/asm.s:850: unrecognized instruction "GF2P8MULB"
        asm: assembly of /tmp/avo2003297476/asm.s failed

@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2022

Codecov Report

Merging #344 (69eb794) into master (a0ea0f3) will decrease coverage by 0.03%.
The diff coverage is 72.61%.

@@            Coverage Diff             @@
##           master     #344      +/-   ##
==========================================
- Coverage   75.95%   75.92%   -0.04%     
==========================================
  Files          65       65              
  Lines       20633    20694      +61     
==========================================
+ Hits        15672    15711      +39     
- Misses       4879     4901      +22     
  Partials       82       82              
Flag Coverage Δ
integration 11.92% <0.00%> (-0.04%) ⬇️
unittests 73.01% <72.61%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/inst/types.go 52.63% <0.00%> (-3.71%) ⬇️
x86/zoptab.go 92.42% <ø> (ø)
build/zinstructions.go 67.65% <60.60%> (-0.03%) ⬇️
internal/load/load.go 95.67% <100.00%> (+0.16%) ⬆️
x86/zctors.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mmcloughlin mmcloughlin marked this pull request as ready for review November 28, 2022 02:44
@mmcloughlin mmcloughlin merged commit 9463235 into master Nov 28, 2022
@mmcloughlin mmcloughlin deleted the add-gfni-tables branch November 28, 2022 02:53
@klauspost
Copy link
Contributor

@mmcloughlin Thanks a bunch. Added it to klauspost/reedsolomon#228 - going in after a bit of testing.

Sorry about the extra work. I will try to catch up on what you did so I can give you a more finished update if I should need it in the future.

@mmcloughlin
Copy link
Owner Author

@mmcloughlin Thanks a bunch. Added it to klauspost/reedsolomon#228 - going in after a bit of testing.

Sorry about the extra work. I will try to catch up on what you did so I can give you a more finished update if I should need it in the future.

Yeah no worries at all! This was always something I needed to work on, and GFNI turned out to be a nice small example to get things going. Having done this I can return to #234 and finally land it.

Great to see it in klauspost/reedsolomon#228 so quickly.

@vsivsi
Copy link
Contributor

vsivsi commented Dec 16, 2022

@mmcloughlin See newly updated PRs #234 and #349. Both caught-up with latest master and adopting the opcodesextra mechanism for adding new instructions. Those two PRs plus this one should bring Avo up to all AVX512 features currently supported by the Go assembler.

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.

4 participants