[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

Yes, the "real builtin" approach seems to be best. For a recent example, 
https://reviews.llvm.org/D49606 added `__shiftright128` as an inline function 
in intrin.h, but that didn't work with MSVC's STL when I moved our declaration 
of `__shiftright128` from intrin.h to intrin0.h. This was fixed by 
https://reviews.llvm.org/D50907 making `__shiftright128` a real builtin.

For the record, MSVC's intrin.h declares ~880 intrinsics (plus many many more 
from immintrin.h etc.) and it includes intrin0.h to get the declarations of 
~130 intrinsics. There is no duplication - each intrinsic is declared in either 
the small intrin0.h or the large intrin.h. MSVC's STL then includes intrin0.h 
in its headers for increased throughput. We occasionally move intrinsics from 
intrin.h to intrin0.h as we discover that they're needed in the STL (as we've 
recently done for `__shiftright128`, `_umul128`, `_BitScanForward[64]`, and 
`_BitScanReverse[64]`).


Repository:
  rC Clang

https://reviews.llvm.org/D53912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: STL_MSFT.
rnk added a comment.

In https://reviews.llvm.org/D53912#1284804, @azharudd wrote:

> I agree. This currently resolves issues with building for ARM64 target using 
> Visual Studio 2017. The missing intrinsics it complains about are already 
> present in intrin.h. We could add those to intrin0.h too but that would 
> duplicate. Please let me know what you think.


I see that more intrinsics are being added as builtins in 
https://reviews.llvm.org/D54046. I think that's a good future direction. We 
should do our best not to add new headers to shadow CRT and SDK headers 
anymore. +@STL_MSFT


Repository:
  rC Clang

https://reviews.llvm.org/D53912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Azharuddin Mohammed via Phabricator via cfe-commits]

azharudd added a comment.

In https://reviews.llvm.org/D53912#1281584, @rnk wrote:

> This sounds like it would defeat what I'm assuming is the intended purpose of 
> intrin0.h, which is to reduce compile time. intrin.h is kind of enormous, and 
> the compile time problems are well-documented. We should investigate what's 
> up with intrin0.h and implement as many builtins as we need to support it.


I agree. This currently resolves issues with building for ARM64 target using 
Visual Studio 2017. The missing intrinsics it complains about are already 
present in intrin.h. We could add those to intrin0.h too but that would 
duplicate. Please let me know what you think.


Repository:
  rC Clang

https://reviews.llvm.org/D53912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This sounds like it would defeat what I'm assuming is the intended purpose of 
intrin0.h, which is to reduce compile time. intrin.h is kind of enormous, and 
the compile time problems are well-documented. We should investigate what's up 
with intrin0.h and implement as many builtins as we need to support it.


Repository:
  rC Clang

https://reviews.llvm.org/D53912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Azharuddin Mohammed via Phabricator via cfe-commits]

azharudd created this revision.
azharudd added reviewers: rnk, mgrang.
Herald added subscribers: jfb, mgorny.

xatomic.h header in VS 2017 includes  instead of  like
before. Adding an intrin0.h header which internally includes  when
compiling for the Windows platform.


Repository:
  rC Clang

https://reviews.llvm.org/D53912

Files:
  lib/Headers/CMakeLists.txt
  lib/Headers/intrin0.h


Index: lib/Headers/intrin0.h
===
--- /dev/null
+++ lib/Headers/intrin0.h
@@ -0,0 +1,30 @@
+/* === intrin0.h --===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+/* xatomic.h in VS 2017 includes  instead of .
+ * If compiling for the Windows platform, we'll internally just include
+ * . */
+
+#ifdef _MSC_VER
+#include 
+#endif /* _MSC_VER */
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -57,6 +57,7 @@
   ia32intrin.h
   immintrin.h
   intrin.h
+  intrin0.h
   inttypes.h
   invpcidintrin.h
   iso646.h


Index: lib/Headers/intrin0.h
===
--- /dev/null
+++ lib/Headers/intrin0.h
@@ -0,0 +1,30 @@
+/* === intrin0.h --===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+/* xatomic.h in VS 2017 includes  instead of .
+ * If compiling for the Windows platform, we'll internally just include
+ * . */
+
+#ifdef _MSC_VER
+#include 
+#endif /* _MSC_VER */
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -57,6 +57,7 @@
   ia32intrin.h
   immintrin.h
   intrin.h
+  intrin0.h
   inttypes.h
   invpcidintrin.h
   iso646.h
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits