Implement avx512bf16 intrinsics #998

kangshan1157 · 2021-02-09T08:52:10Z

Implement all avx512bf16 intrinsic APIs.

rust-highfive · 2021-02-09T08:52:13Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

bjorn3 · 2021-02-09T08:55:31Z

crates/core_arch/src/x86/avx512bf16.rs

+            "vcvtneps2bf16 {1} {{k1}}, {0}",
+            in(xmm_reg) a,
+            inout(xmm_reg) src => result,
+            in("edi") mask,


You need to use at&t syntax for now. Some older LLVM versions that are still supported by rustc don't correctly support intel syntax.

Thanks @bjorn3 for your suggestion. I have updated it.

kangshan1157 · 2021-02-09T10:26:51Z

I tested and debugged my code with launching the command "TARGET=x86_64-unknown-linux-gnu ci/run.sh" on the cooperlake. Are any other tests required?

Amanieu · 2021-02-09T10:50:46Z

crates/core_arch/src/x86/avx512bf16.rs

+            in(xmm_reg) a,
+            lateout(xmm_reg) result,
+            options(att_syntax)
+        );


Why is this implemented using asm! instead of LLVM intrinsics?

Amanieu · 2021-02-09T10:52:01Z

You should fix the CI errors.

The asm! is causing the doc build to fail, but you can fix that by #[cfg]ing it out for non-x86.
The verifier doesn't support the newly added types.

Amanieu · 2021-02-09T11:57:35Z

crates/core_arch/src/x86/avx512bf16.rs

+    : "={xmm0}"(result) 
+    : "{xmm0}"(a)
+    : "xmm0" 
+    :);


Please don't use llvm_asm!, it is going to be deprecated soon.

Instead just wrap the asm! in a cfg_if and provide a dummy unreachable!() implementation on other platforms.

Thanks for your review. I have updated the code following your suggestions.

bjorn3 · 2021-02-09T13:23:26Z

crates/core_arch/Cargo.toml

@@ -25,3 +25,6 @@ maintenance = { status = "experimental" }
 [dev-dependencies]
 stdarch-test = { version = "0.*", path = "../stdarch-test" }
 std_detect = { version = "0.*", path = "../std_detect" }
+
+[dependencies]
+cfg-if = "0.1.10"


core_arch can't have any dependencies as it is included in libcore, which everyone else depends on, including cfg-if.

OK. Then is there any way to use cfg_if?

bjorn3 · 2021-02-09T13:28:54Z

crates/core_arch/src/x86/avx512bf16.rs

+    cfg_if! {
+        if #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] {
+            let mut result:__m128bh;
+            asm!(
+                    "vcvtneps2bf16 {0}, {1}",
+                    in(xmm_reg) a,
+                    lateout(xmm_reg) result,
+                    options(att_syntax)
+                );
+            result
+        }
+        else {
+            unreachable!()
+        }
+    }


Suggested change

cfg_if! {

if #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] {

let mut result:__m128bh;

asm!(

"vcvtneps2bf16 {0}, {1}",

in(xmm_reg) a,

lateout(xmm_reg) result,

options(att_syntax)

);

result

}

else {

unreachable!()

}

}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]

{

let mut result:__m128bh;

asm!(

"vcvtneps2bf16 {0}, {1}",

in(xmm_reg) a,

lateout(xmm_reg) result,

options(att_syntax)

);

result

}

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]

{

unreachable!()

}

This should work instead of cfg-if.

Thank you very much. It works. :)

kangshan1157 · 2021-02-09T13:51:27Z

The CI tests for i686-unknown-linux-gnu are still failed. It always failed with the error "register class xmm_reg requires the sse target feature". I added "target_feature(enable = "sse"), but it didn't work.
It seems that asm! doesn't support well on that target. Do I have to use "llvm_asm!" for x86 target?

Amanieu · 2021-02-09T21:52:01Z

xmm_reg can't be used on i586 because it doesn't support SSE (enabling it with #[target_feature] doesn't help, it needs to be globally enabled).

We really should be using LLVM intrinsics for this instead of inline asm. From what I understand, the main issue is that we can't represent a i1x4 with Rust code. Support for this needs to be added to rustc.

minybot · 2021-02-09T22:47:09Z

We really should be using LLVM intrinsics for this instead of inline asm. From what I understand, the main issue is that we can't represent a i1x4 with Rust code. Support for this needs to be added to rustc.
Btw, how difficult is it to add i1 to rustc?

Amanieu · 2021-02-09T22:51:00Z

I would recommend asking the compiler team on Zulip.

I think this would require defining a wrapper struct like this:

#[repr(simd_mask(4))]
struct i1x4(u8);

Then you would need to adjust the calling convention code to handle this type.

But I'm sure the compiler team will have a clearer idea of what would be required.

kangshan1157 · 2021-02-10T00:00:25Z

Thanks for all of your advices. You are right and we should call LLVM intrinsics to avoid the issues. Actually I asked the question in rust github several days ago. But they didn't give me the final answer. I will try to ask it on Zulip.
Moreover I will remove those asm implemented functions in this PR and recover them once i1x4 is supported by rustc.

minybot · 2021-02-10T00:09:58Z

Thanks for all of your advices. You are right and we should call LLVM intrinsics to avoid the issues. Actually I asked the question in rust github several days ago. But they didn't give me the final answer. I will try to ask it on Zulip.
Moreover I will remove those asm implemented functions in this PR and recover them once i1x4 is supported by rustc.
In some of my case to implement avx512f_avx512vl. LLVM use i1x2. I think rustc need to support i1 to solve those issues.

kangshan1157 · 2021-02-10T00:19:44Z

Thanks for all of your advices. You are right and we should call LLVM intrinsics to avoid the issues. Actually I asked the question in rust github several days ago. But they didn't give me the final answer. I will try to ask it on Zulip.
Moreover I will remove those asm implemented functions in this PR and recover them once i1x4 is supported by rustc.
In some of my case to implement avx512f_avx512vl. LLVM use i1x2. I think rustc need to support i1 to solve those issues.

OK. I will also mention this to them.
Moreover CI tests for x86_64-unknown-linux-gnu and i586-unkonwn-linux-gnu failed because of the error "thread 'core_arch::x86::avx512bf16::assert__mm512_maskz_cvtneps_pbh_vcvtneps2bf16' panicked at 'failed to find instruction vcvtneps2bf16 in the disassembly', crates/stdarch-test/src/lib.rs:148:9". How can I resolve this? I tried them on my test machine and they work normally.

minybot · 2021-02-10T00:35:14Z

Moreover CI tests for x86_64-unknown-linux-gnu and i586-unkonwn-linux-gnu failed because of the error "thread 'core_arch::x86::avx512bf16::assert__mm512_maskz_cvtneps_pbh_vcvtneps2bf16' panicked at 'failed to find instruction vcvtneps2bf16 in the disassembly', crates/stdarch-test/src/lib.rs:148:9". How can I resolve this? I tried them on my test machine and they work normally.

thread 'core_arch::x86::avx512bf16::assert__mm256_mask_dpbf16_ps_vdpbf16ps' panicked at 'failed to find instruction vdpbf16ps in the disassembly', crates/stdarch-test/src/lib.rs:148:9

When rustc tried to generate your code, it did not use vcvtne2ps2bf16 instruction.
---- core_arch::x86::avx512bf16::assert__mm512_cvtne2ps_pbh_vcvtne2ps2bf16 stdout ----
disassembly for stdarch_test_shim__mm512_cvtne2ps_pbh_vcvtne2ps2bf16:
0: lea 0x1fdfe8(%rip),%rax # 2edcdf <anon.e2d3528a6cfe9ff3fa80755a098d527b.13.llvm.18001095552927935278+0x34>
1: lea 0x611312(%rip),%rcx # 701010 <_ZN12stdarch_test11_DONT_DEDUP17h0d83dd6e7a766ff3E>
2: mov %rax,(%rcx)
3: (bad)
4: rol $0xf,%ebx
5: (bad)
6: test %al,(%rax)
7: add %al,(%rax)
8:
Usually, it is relating to your code error.
Do you try to compile your code to assembly?

kangshan1157 · 2021-02-10T00:38:41Z

Moreover CI tests for x86_64-unknown-linux-gnu and i586-unkonwn-linux-gnu failed because of the error "thread 'core_arch::x86::avx512bf16::assert__mm512_maskz_cvtneps_pbh_vcvtneps2bf16' panicked at 'failed to find instruction vcvtneps2bf16 in the disassembly', crates/stdarch-test/src/lib.rs:148:9". How can I resolve this? I tried them on my test machine and they work normally.

thread 'core_arch::x86::avx512bf16::assert__mm256_mask_dpbf16_ps_vdpbf16ps' panicked at 'failed to find instruction vdpbf16ps in the disassembly', crates/stdarch-test/src/lib.rs:148:9

When rustc tried to generate your code, it did not use vcvtne2ps2bf16 instruction.
---- core_arch::x86::avx512bf16::assert__mm512_cvtne2ps_pbh_vcvtne2ps2bf16 stdout ----
disassembly for stdarch_test_shim__mm512_cvtne2ps_pbh_vcvtne2ps2bf16:
0: lea 0x1fdfe8(%rip),%rax # 2edcdf <anon.e2d3528a6cfe9ff3fa80755a098d527b.13.llvm.18001095552927935278+0x34>
1: lea 0x611312(%rip),%rcx # 701010 <_ZN12stdarch_test11_DONT_DEDUP17h0d83dd6e7a766ff3E>
2: mov %rax,(%rcx)
3: (bad)
4: rol $0xf,%ebx
5: (bad)
6: test %al,(%rax)
7: add %al,(%rax)
8:
Usually, it is relating to your code error.
Do you try to compile your code to assembly?

Yes, I tried it on my test machine. They are generated correctly. It seems that vcvtneps2bf16 is not supported by the compiler in the CI environment.

minybot · 2021-02-10T00:46:18Z

You can remove i586 in .github/workflows/main.yml temporary for test.

Yes, I tried it on my test machine. They are generated correctly. It seems that vcvtneps2bf16 is not supported by the compiler in the CI environment.

kangshan1157 · 2021-02-10T00:52:29Z

I would recommend asking the compiler team on Zulip.

I think this would require defining a wrapper struct like this:
#[repr(simd_mask(4))]
struct i1x4(u8);
Then you would need to adjust the calling convention code to handle this type.

But I'm sure the compiler team will have a clearer idea of what would be required.

Hi

I would recommend asking the compiler team on Zulip.

I think this would require defining a wrapper struct like this:
#[repr(simd_mask(4))]
struct i1x4(u8);
Then you would need to adjust the calling convention code to handle this type.

But I'm sure the compiler team will have a clearer idea of what would be required.

Hi Amanieu, does bf16 related feature supported by the rust compiler in CI environment? The CI tests failed because failed to find instruction vdpbf16ps in the disassembly', crates/stdarch-test/src/lib.rs:148:9

Amanieu · 2021-02-10T01:32:25Z

The (bad) in the disassembly is actually because the disassembler is too old and doesn't recognize the instruction. LLVM is still generating the correct code.

Amanieu · 2021-02-10T01:33:27Z

Maybe try upgrading the Dockerfile from ubuntu:18.04 to ubuntu:20.04.

kangshan1157 · 2021-02-10T01:36:27Z

Maybe try upgrading the Dockerfile from ubuntu:18.04 to ubuntu:20.04.

How to upgrade it? Do I have the permission to do it?

Amanieu · 2021-02-10T01:38:23Z

Just modify the Dockerfile in ci/docker for all platforms where CI fails.

Amanieu · 2021-02-10T01:40:38Z

If the MSVC targets still give you trouble then you can disable the test for those targets in the source code by modifying the cfg_attr to exclude target_env = "msvc".

…ort BF16

…bf16

kangshan1157 · 2021-02-10T13:09:58Z

Just modify the Dockerfile in ci/docker for all platforms where CI fails.

@Amanieu Thank you very much for your help.
For x86_64-unknown-linux-gnu-emulated target, as it doesn't allow skipping tests, those avx512bf16 related tests will fail. So I force to skip those tests through adding "SKIP_TESTS" environment variable. Now all of the CI tests can pass.

If we want to run those bf16 tests in CI, another emulator needs to be set up. From what I learned, only cooper lake supports avx512bf16. If we start the emulator through "sde64 -cpx" to simulate cooper lake, then the other tests such as vnni will fail. So I suggest to start a separate emulator for bf16 tests only if CI doesn't allow to skip any tests.

Amanieu · 2021-02-10T17:08:55Z

If we start the emulator through "sde64 -cpx" to simulate cooper lake, then the other tests such as vnni will fail.

I thought cooper lake did support vnni?

In any case, I'm happy to merge this if you don't think you can get the emulator working.

kangshan1157 · 2021-02-10T23:28:47Z

If we start the emulator through "sde64 -cpx" to simulate cooper lake, then the other tests such as vnni will fail.

I thought cooper lake did support vnni?

In any case, I'm happy to merge this if you don't think you can get the emulator working.

Sorry for my typo, cooper lake doesn't support vbmi,vbmi2, so those tests will fail. Could you please help to merge this PR? I will add a new emulator target for bf16 tests later.

Implement avx512bf16 intrinsics

5e8bfdd

rust-highfive assigned Amanieu Feb 9, 2021

bjorn3 reviewed Feb 9, 2021

View reviewed changes

Changed to use at&t syntax

8a3eda2

Amanieu reviewed Feb 9, 2021

View reviewed changes

Fix CI issues for new avx512bf16 intrinsics

043f787

Amanieu reviewed Feb 9, 2021

View reviewed changes

Use asm instead of llvm_asm and use cfg_if to make CI tests pass

cf75c96

bjorn3 reviewed Feb 9, 2021

View reviewed changes

Remove cfg_if from core_arch

7e60928

Remove asm related intrinsic implementations

625af62

Upgrade ubuntu docker for BF16 support

9687930

kangshan1157 added 9 commits February 10, 2021 09:53

Upgrade ubuntu for x86_64-unknown-linux-gnu-emulated in order to supp…

20313e3

…ort BF16

Update intel SDE emaulator for BF16 support

273f3db

Fix sde command

fe53b83

Fix sde command

4c62516

For debug purpose

c2782d9

For debug purpose

4d806df

For debug purpose

c98f454

For debug purpose

df20dc2

Skip avx512bf16 tests on emulator, as the emulator failed to support …

dc13e89

…bf16

Amanieu merged commit a06cb4c into rust-lang:master Feb 10, 2021

tarcieri mentioned this pull request Feb 13, 2021

ARMv8 Cryptography Extensions missing? #1003

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement avx512bf16 intrinsics #998

Implement avx512bf16 intrinsics #998

kangshan1157 commented Feb 9, 2021

rust-highfive commented Feb 9, 2021

bjorn3 Feb 9, 2021

kangshan1157 Feb 9, 2021

kangshan1157 commented Feb 9, 2021

Amanieu Feb 9, 2021

Amanieu commented Feb 9, 2021

Amanieu Feb 9, 2021

kangshan1157 Feb 9, 2021

bjorn3 Feb 9, 2021

kangshan1157 Feb 9, 2021

bjorn3 Feb 9, 2021

kangshan1157 Feb 9, 2021

kangshan1157 commented Feb 9, 2021 •

edited

Loading

Amanieu commented Feb 9, 2021

minybot commented Feb 9, 2021

Amanieu commented Feb 9, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

Amanieu commented Feb 10, 2021

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

Amanieu commented Feb 10, 2021

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021 •

edited

Loading

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

Implement avx512bf16 intrinsics #998

Implement avx512bf16 intrinsics #998

Conversation

kangshan1157 commented Feb 9, 2021

rust-highfive commented Feb 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangshan1157 commented Feb 9, 2021

Choose a reason for hiding this comment

Amanieu commented Feb 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangshan1157 commented Feb 9, 2021 • edited Loading

Amanieu commented Feb 9, 2021

minybot commented Feb 9, 2021

Amanieu commented Feb 9, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

minybot commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

Amanieu commented Feb 10, 2021

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

Amanieu commented Feb 10, 2021

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021 • edited Loading

Amanieu commented Feb 10, 2021

kangshan1157 commented Feb 10, 2021

kangshan1157 commented Feb 9, 2021 •

edited

Loading

kangshan1157 commented Feb 10, 2021 •

edited

Loading