[PATCH] D47142: [x86] invpcid intrinsic

Gabor Buella via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 21 12:04:29 PDT 2018


GBuella added inline comments.


================
Comment at: lib/Headers/intrin.h:196
+ */
 void __cdecl _invpcid(unsigned int, void *);
+#endif
----------------
rnk wrote:
> GBuella wrote:
> > GBuella wrote:
> > > rnk wrote:
> > > > craig.topper wrote:
> > > > > @rnk, what's the right thing to do here?
> > > > What problems does this redeclaration cause?
> > > Now that I think about it, none.
> > Also, I think this could be added as TARGET_HEADER_BUILTIN..."intrin.h", ALL_MS_LANGUAGES into BuiltinsX86.def
> > And removed from here.
> > Right?
> It could be, but then you'd need to implement it as a builtin instead of having invpcidintrin.h, right?
Meanwhile, I just realized, we add the `!defined(_MSC_VER)` condition around the inclusion of all such header files, as I did with `invpcidintrin.h`.
Does clang always define _MSC_VER on Windows? I mean, always in cases where one would want to include the MSVC likes`intrin.h`?
In that case, these are completely independent things. Also, I could just add both builtins `_invpcid` with the ALL_MS_LANGUAGES thing, and the `__builtin_ia32_invpcid` with the usual clang/GCC convention of adding separate feature flags (both would be lowered to the same LLVM intrinsic).
BTW, if we stop adding feature flags for such intrinsics, we probably have to convince GCC to do the same, as we love that compatibility.


Repository:
  rC Clang

https://reviews.llvm.org/D47142





More information about the cfe-commits mailing list