Skip to content

[x86 & wasm] Split up double saturating-narrows from i32 #7280

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.

Sign up for GitHub

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

Jump to bottom
Merged
rootjalex merged 7 commits into from
Jan 20, 2023
Merged

[x86 & wasm] Split up double saturating-narrows from i32 #7280

rootjalex merged 7 commits into from
Jan 20, 2023

Conversation

@rootjalex
Copy link
Member

@rootjalex rootjalex commented Jan 16, 2023

We can get much better codegen for double saturating narrows from i32 on x86 and wasm. HVX and ARM backends both already do this. Also added tests to simd_op_check

Fixes #7069

Here's an example from x86 (wasm is similar):

Var x("x");
ImageParam in(Int(32), 1);
Func f("f");

f(x) = u8_sat(in(x));
f.vectorize(x, 32);

Target x86("x86-64-linux-avx-avx2-fma-sse41");
f.compile_to_assembly("vpack.asm", f.infer_arguments(), x86);

Previously:

	vpminsd	-96(%rax,%rcx,4), %ymm0, %ymm2
	vpminsd	-64(%rax,%rcx,4), %ymm0, %ymm3
	vpminsd	-32(%rax,%rcx,4), %ymm0, %ymm4
	vpminsd	(%rax,%rcx,4), %ymm0, %ymm5
	vpmaxsd	%ymm1, %ymm2, %ymm2
	vpmaxsd	%ymm1, %ymm3, %ymm3
	vpackusdw	%ymm3, %ymm2, %ymm2
	vpmaxsd	%ymm1, %ymm4, %ymm3
	vpmaxsd	%ymm1, %ymm5, %ymm4
	vpackusdw	%ymm4, %ymm3, %ymm3
	vpermq	$216, %ymm3, %ymm3              # ymm3 = ymm3[0,2,1,3]
	vpermq	$216, %ymm2, %ymm2              # ymm2 = ymm2[0,2,1,3]
	vpackuswb	%ymm3, %ymm2, %ymm2
	vpermq	$216, %ymm2, %ymm2              # ymm2 = ymm2[0,2,1,3]
	vmovdqu	%ymm2, (%r8,%rcx)

Now:

	vmovdqu	-96(%rax,%rcx,4), %ymm0
	vmovdqu	-32(%rax,%rcx,4), %ymm1
	vpackssdw	-64(%rax,%rcx,4), %ymm0, %ymm0
	vpackssdw	(%rax,%rcx,4), %ymm1, %ymm1
	vpermq	$216, %ymm0, %ymm0              # ymm0 = ymm0[0,2,1,3]
	vpermq	$216, %ymm1, %ymm1              # ymm1 = ymm1[0,2,1,3]
	vpackuswb	%ymm1, %ymm0, %ymm0
	vpermq	$216, %ymm0, %ymm0              # ymm0 = ymm0[0,2,1,3]
	vmovdqu	%ymm0, (%r8,%rcx)

@steven-johnson
Copy link
Contributor

steven-johnson commented Jan 17, 2023

https://buildbot.halide-lang.org/master/#/builders/42/builds/693 looks like perhaps a real failure

steven-johnson
steven-johnson requested changes Jan 17, 2023
View reviewed changes
Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

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

https://buildbot.halide-lang.org/master/#/builders/42/builds/693 looks like a problem

@rootjalex
Copy link
Member Author

rootjalex commented Jan 17, 2023

@steven-johnson I see the same failure on #7279 , so I don't think either PR is responsible for the failure
https://buildbot.halide-lang.org/master/#/builders/42/builds/692

@steven-johnson
Copy link
Contributor

steven-johnson commented Jan 17, 2023 •
edited
Loading

@steven-johnson I see the same failure on #7279 , so I don't think either PR is responsible for the failure https://buildbot.halide-lang.org/master/#/builders/42/builds/692

great, it's probably another LLVM injection :-/

Let me try to confirm that first

@steven-johnson
Copy link
Contributor

steven-johnson commented Jan 17, 2023

The predicated-load failure isn't happening for me locally with top-of-tree LLVM, so maybe it's a temporary flake; I'm forcing rebuilds on the x64 bots to see if it recurs

@rootjalex
Copy link
Member Author

rootjalex commented Jan 19, 2023

Only failing test appears unrelated. @steven-johnson think it’s good to go?

@steven-johnson
Copy link
Contributor

steven-johnson commented Jan 19, 2023

Failure is vectorized_gpu_allocation, which I've never seen before as a flake or even an ordinary failure, so let me retry it just a bit first.

@steven-johnson
Copy link
Contributor

steven-johnson commented Jan 19, 2023

The failure is now in our old friend, correctness_atomics, aka "Mr. Flaky", so I think we're good to go

@rootjalex rootjalex merged commit bafd60f into main Jan 20, 2023
@rootjalex rootjalex deleted the rootjalex/x86-double-sat branch January 20, 2023 18:03
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
@rootjalex @steven-johnson @ardier
[x86 & wasm] Split up double saturating-narrows from i32 (halide#7280)
3ed06f7 
* better x86 double sat-cast + add test

* fix wasm too + test

Co-authored-by: Steven Johnson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steven-johnson steven-johnson steven-johnson approved these changes

@abadams abadams abadams approved these changes

Assignees

No one assigned

Labels

performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[x86] Codegen should split up double saturating-narrows

4 participants

@rootjalex @steven-johnson @abadams