Support AVX512_BITALG instructions and relax base register req in VM operands #199

vsivsi · 2021-09-08T21:48:21Z

This PR updates the AVX512 branch only, and brings the following:

Relaxes the current requirement that VM operands in VPSCATTER/GATHER have a base register.
- This is not required by AVX512 or the golang assembler, as discussed here: SCATTER/GATHER Vector memory operand validation is too strict #193
Adds support for AVX512_BITALG instructions / forms.
- Adds VPOPCNTB/W byte/word forms of VPOPCNT, including AVX512VL 128/256-bit wide (xmm/ymm) forms
- Adds AVX512VL 128/256-bit wide (xmm/ymm) forms to the existing supported AVX512_VPOPCNTDQ instructions.
- Adds new instruction VPSHUFBITQMB with its AVX512VL 128/256-bit wide (xmm/ymm) forms.
- Support for these instructions/forms was hand edited into internal/data/x86_64.xml as that seemed the path of least resistance, and scripts/generate was run to regenerate build/zinstructions.go et al.

Additionally it:

Merges in all changes from the current master branch
Updates CI for go1.17 (from go1.17 branch)

…hlin#165) This makes it easier to debug avogen: when you emit invalid syntax, you can inspect the generated file to determine what went wrong, instead of having only gofmt's error to work with.

…popcnt_VL

Add necessary feature checks to the dot and geohash examples to prevent illegal instruction errors. Fixes mmcloughlin#170 mmcloughlin#153

Replaces gobin with the preferred tools.mod solution for pinning tool dependencies. Updates mmcloughlin#166

Adds a method to the Attribute type to test for each flag in the textflag.h header.

Updates mmcloughlin#156

…#182) Many of the instruction functions correctly match the size of immediate values, but they only accept unsigned immediates. This PR fixes the operand check functions for intermediate types to also accept the signed variants. Fixes mmcloughlin#181

Currently `avo` uses `BP` as a standard general-purpose register. However, `BP` is used for the frame pointer and should be callee-save. Under some circumstances, the Go assembler will do this automatically, but not always. At the moment `avo` can produce code that clobbers the `BP` register. Since Go 1.16 this code will also fail a new `go vet` check. This PR provides a (currently sub-optimal) fix for the issue. It introduces an `EnsureBasePointerCalleeSaved` pass which will check if the base pointer is written to by a function, and if so will artificially ensure that the function has a non-zero frame size. This will trigger the Go assembler to automatically save and restore the BP register. In addition, we update the `asmdecl` tool to `asmvet`, which includes the `framepointer` vet check. Updates mmcloughlin#156

) Updates mmcloughlin#156

Update github.com/klauspost/compress version. Add github.com/klauspost/reedsolomon and github.com/minio/md5-simd. Add `-mod=mod` to commands due to golang/go#44129.

Required updates to the thirdparty test suite due to modules changes in go toolchain.

Link to Filippo's live stream of rewriting the filippo.io/edwards25519 assembly with avo. Reformat the "learn more" links in a list.

Restrict permissions of github token. Pin action versions. Following advice in briansmith/untrusted#50.

Upgrades codecov-action in order to fix codecov errors CI, for example: https://github.com/mmcloughlin/avo/runs/3389170262?check_suite_focus=true#step:10:11 See codecov/codecov-action#322.

…popcnt_VL

codecov-commenter · 2021-09-08T21:52:48Z

Codecov Report

Merging #199 (fdca336) into avx512 (d60cc02) will decrease coverage by 4.34%.
The diff coverage is 97.15%.

@@            Coverage Diff             @@
##           avx512     #199      +/-   ##
==========================================
- Coverage   75.57%   71.22%   -4.35%     
==========================================
  Files          58       59       +1     
  Lines       79634    90787   +11153     
==========================================
+ Hits        60184    64666    +4482     
- Misses      19294    25963    +6669     
- Partials      156      158       +2

Flag	Coverage Δ
integration	`3.20% <17.63%> (+0.53%)`	⬆️
unittests	`70.26% <96.29%> (-4.54%)`	⬇️

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

Impacted Files	Coverage Δ
attr/attr.go	`100.00% <ø> (ø)`
build/zinstructions.go	`1.61% <ø> (-0.09%)`	⬇️
gotypes/components.go	`92.37% <ø> (+0.70%)`	⬆️
internal/gen/gen.go	`0.00% <0.00%> (ø)`
pass/pass.go	`51.61% <ø> (+10.43%)`	⬆️
reg/types.go	`83.20% <ø> (+1.49%)`	⬆️
reg/x86.go	`100.00% <ø> (ø)`
x86/zoptab.go	`0.00% <ø> (ø)`
x86/zctors.go	`95.21% <98.14%> (+0.25%)`	⬆️
attr/ztextflag.go	`100.00% <100.00%> (ø)`
... and 62 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d60cc02...fdca336. Read the comment docs.

vsivsi · 2021-09-08T22:01:22Z

I'm not sure why the lint CI run is exiting with retval 1. But it looks like lint has been failing for the AVX512 branch all along.

It also looks like 'lint' and 'thirdparty' are currently failing on the go1.17 branch, and since I merged that in... Also failing here.

Perhaps it was a mistake to do that, happy to revert the CI to 1.16 on this branch if moving to 1.17 is complicated.

vsivsi · 2021-11-15T20:42:09Z

@mmcloughlin What needs to happen to get this merged? This is the last thing blocking me from moving off my fork and back to Avo master. Specifically this addresses #193 and #20 (comment). If it would be helpful to have me resubmit this as a PR against the master branch, I'd be happy to do that.

mmcloughlin · 2021-11-15T21:05:58Z

Can we split this into two?

For the BITALG stuff, I'd like any hand edits to be recorded as patch files. It's looking unlikely that opcodes will ever be updated, but in theory, it could be. Basically, we need to get to the point where running the dl.sh script produces the datafiles. I'd suggest a PR that:

adds a internal/data/patches/avx512bitalg.patch
update internal/data/dl.sh script to add an applypatch function
the applypatch function should call the patch command with a given patch file
echo some output to record the patch in the internal/data/README.md file

For the vm operands, it basically looks good but it needs more tests. Let me take a look at this.

vsivsi · 2021-11-15T21:41:44Z

Sure thing! Thanks for the quick response. Here's a new PR for the S/G mem op validation: #233

vsivsi · 2021-11-15T21:59:02Z

And another PR with the requested patch file. #234

If it's not too much to ask, I'll leave the patching mechanics and opcodes rebuild up to you, since you seem to have a vision for how that should all work that would probably take longer to communicate than to just implement.

vsivsi · 2021-11-15T22:03:11Z

With #233 and #234 this PR is now defunct, so I'm closing it.

mmcloughlin · 2021-11-15T22:46:14Z

If it's not too much to ask, I'll leave the patching mechanics and opcodes rebuild up to you, since you seem to have a vision for how that should all work that would probably take longer to communicate than to just implement.

For sure.

josharian and others added 30 commits January 3, 2021 19:29

internal/cmd/avogen: write output file even when gofmt fails (mmcloug…

599bdd1

…hlin#165) This makes it easier to debug avogen: when you emit invalid syntax, you can inspect the generated file to determine what went wrong, instead of having only gofmt's error to work with.

ci: bump to go 1.16

c63f459

module auto in bootstrap

f592679

go env

14d5f9f

use tools.mod for tools install

899a3ca

Merge commit '899a3ca3ff8315036b105e73effd6b2ee269112b' into avx512_v…

45b858c

…popcnt_VL

Update/regen with VL forms of VPOPCNTD/Q

04add25

Added VPOPCNTB/W BITALG inst + VL forms

f19e503

Fixed VPOPCNTB/W broadcast, Added VPSHUFBITQMB

2be30b1

examples: cpu feature checks (mmcloughlin#171)

8f3c4ea

Add necessary feature checks to the dot and geohash examples to prevent illegal instruction errors. Fixes mmcloughlin#170 mmcloughlin#153

all: upgrade direct dependencies (mmcloughlin#172)

3f5da8f

script: use tools.mod for bootstrap (mmcloughlin#175)

e5c9b4e

Replaces gobin with the preferred tools.mod solution for pinning tool dependencies. Updates mmcloughlin#166

attr: generate code from textflag.h (mmcloughlin#176)

c5faaae

attr: flag test methods (mmcloughlin#177)

e9f28ca

Adds a method to the Attribute type to test for each flag in the textflag.h header.

ci: codecov informational mode (mmcloughlin#179)

060ad41

pass: support priority in register allocator (mmcloughlin#180)

3622eb0

Updates mmcloughlin#156

pass: de-prioritize base pointer in register allocation (mmcloughlin#184

c32f24f

) Updates mmcloughlin#156

tests/thirdparty: update/add packages, fix modules (mmcloughlin#186)

f798093

Update github.com/klauspost/compress version. Add github.com/klauspost/reedsolomon and github.com/minio/md5-simd. Add `-mod=mod` to commands due to golang/go#44129.

Merge upstream changes in v0.2.0

cb1947b

ci: bump to go 1.16 (mmcloughlin#166)

6bad393

Required updates to the thirdparty test suite due to modules changes in go toolchain.

doc: link to Filippo's live stream in README (mmcloughlin#187)

ec9535c

Link to Filippo's live stream of rewriting the filippo.io/edwards25519 assembly with avo. Reformat the "learn more" links in a list.

ci: github actions hardening (mmcloughlin#190)

3a219c8

Restrict permissions of github token. Pin action versions. Following advice in briansmith/untrusted#50.

Relax ismv() to allow omission of Base register

b2b755d

Allow VM without Base in VerifyMemOperands

fefe3f2

ci: upgrade codecov action (mmcloughlin#196)

b935256

Upgrades codecov-action in order to fix codecov errors CI, for example: https://github.com/mmcloughlin/avo/runs/3389170262?check_suite_focus=true#step:10:11 See codecov/codecov-action#322.

ci: bump go version

c3ee58c

Merge commit 'b935256fa53858859d608444d9f1bde3d952022f' into avx512_v…

8b12b38

…popcnt_VL

Merge branch 'avx512_vpopcnt_VL' into relax-isvm

17f12b0

Merge commit 'c3ee58c2a4e867b3cb026b7eb0438e330b568016' into relax-isvm

fdca336

vsivsi changed the title ~~Support AVX512_BITALG instructions and relax base register req in VM operands (#193)~~ Support AVX512_BITALG instructions and relax base register req in VM operands Sep 8, 2021

mmcloughlin mentioned this pull request Nov 7, 2021

all: support AVX-512 #20

Closed

vsivsi mentioned this pull request Nov 15, 2021

Relax validation of SCATTER/GATHER memory ops to spec, Fixes #193 #233

Open

vsivsi mentioned this pull request Nov 15, 2021

Add BITALG and full VPOPCNTDQ instruction support #234

Closed

vsivsi closed this Nov 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support AVX512_BITALG instructions and relax base register req in VM operands #199

Support AVX512_BITALG instructions and relax base register req in VM operands #199

vsivsi commented Sep 8, 2021

codecov-commenter commented Sep 8, 2021 •

edited

Loading

vsivsi commented Sep 8, 2021 •

edited

Loading

vsivsi commented Nov 15, 2021 •

edited

Loading

mmcloughlin commented Nov 15, 2021

vsivsi commented Nov 15, 2021

vsivsi commented Nov 15, 2021

vsivsi commented Nov 15, 2021

mmcloughlin commented Nov 15, 2021

Support AVX512_BITALG instructions and relax base register req in VM operands #199

Support AVX512_BITALG instructions and relax base register req in VM operands #199

Conversation

vsivsi commented Sep 8, 2021

codecov-commenter commented Sep 8, 2021 • edited Loading

Codecov Report

vsivsi commented Sep 8, 2021 • edited Loading

vsivsi commented Nov 15, 2021 • edited Loading

mmcloughlin commented Nov 15, 2021

vsivsi commented Nov 15, 2021

vsivsi commented Nov 15, 2021

vsivsi commented Nov 15, 2021

mmcloughlin commented Nov 15, 2021

codecov-commenter commented Sep 8, 2021 •

edited

Loading

vsivsi commented Sep 8, 2021 •

edited

Loading

vsivsi commented Nov 15, 2021 •

edited

Loading