[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Discussed with the team different approaches and suggesting https://reviews.llvm.org/D61621 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44387/new/ https://reviews.llvm.org/D44387 ___ cfe-commits mailing list cfe-

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Having in bitcode something like `@llvm.x86.encls.64` is better than inline assembly because we understand the meaning of the bitcode while we don't parse assembly and have a very limited understanding of what it is doing. The use case we need to support is basically

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added a comment. Your understanding is correct. As far as testing I think the existing testing in the original patches is sufficient. I'm not sure I understand how a target specific intrinsic that only works on x86 in the bitcode is substantially better than inline assembly. Do y

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Thanks for the insightful explanation, Craig. As far as I understand, implementing intrinsics with builtins is possible but it is more complex and wasn't providing enough value to prefer it over inline assembly. If that is correct, I'd like to revive the abandoned imple

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added a comment. The main reason that we used inline assembly is due to the fixed register allocation for these instructions. We would have had to write out the register rules in the backend as a special case as seen in the getInstrWFourImplicitOps in https://reviews.llvm.org/D4438

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. I see you've ended up implementing the intrinsics with inline assembly but are there any reasons not to do it with builtins like in this patch? The problem with inline assembly is that for some Apple platforms we require developers to emit bitcode. And inline assembly i

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella added a comment. Here is a variation on this, using inline asm: https://reviews.llvm.org/D46431 https://reviews.llvm.org/D44387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added a comment. I didn't see an answer to the inline assembly question. Comment at: lib/Headers/pconfigintrin.h:31 + +#define MKTME_KEY_PROGRAM 0x0001 + craig.topper wrote: > This doesn't match the name used by gcc. It also needs to start with

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella updated this revision to Diff 144113. https://reviews.llvm.org/D44387 Files: include/clang/Basic/BuiltinsX86.def include/clang/Driver/Options.td lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h lib/CodeGen/CGBuiltin.cpp lib/Headers/CMakeLists.txt lib/Headers/cpuid.h lib/H

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella updated this revision to Diff 144112. https://reviews.llvm.org/D44387 Files: include/clang/Basic/BuiltinsX86.def include/clang/Driver/Options.td lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h lib/CodeGen/CGBuiltin.cpp lib/Headers/CMakeLists.txt lib/Headers/cpuid.h lib/H

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added inline comments. Comment at: lib/Headers/pconfigintrin.h:28 + +#ifndef _PCONFIGINTRIN_H +#define _PCONFIGINTRIN_H I think all our other headers use double underscore here. Comment at: lib/Headers/sgxintrin.h:28 + +#ifndef _SG

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

craig.topper added a comment. I see that gcc implements all of this with wrappers around inline assembly. Any reason we couldn't just do that? Comment at: lib/Headers/pconfigintrin.h:31 + +#define MKTME_KEY_PROGRAM 0x0001 + This doesn't match the name used

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella updated this revision to Diff 143640. https://reviews.llvm.org/D44387 Files: include/clang/Basic/BuiltinsX86.def include/clang/Driver/Options.td lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h lib/CodeGen/CGBuiltin.cpp lib/Headers/CMakeLists.txt lib/Headers/cpuid.h lib/H

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella updated this revision to Diff 143639. GBuella added a comment. Rebased the patch. Added pconfig to Icelake Server. https://reviews.llvm.org/D44387 Files: include/clang/Basic/BuiltinsX86.def include/clang/Driver/Options.td lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h lib/C

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

GBuella created this revision. GBuella added reviewers: craig.topper, zvi. Herald added subscribers: cfe-commits, mgorny. Introduce pconfig and SGX related intrinsics. Repository: rC Clang https://reviews.llvm.org/D44387 Files: include/clang/Basic/BuiltinsX86.def include/clang/Driver/Opt