[PATCH] D22181: Add runtime support for __cpu_model (__builtin_cpu_supports)

[Eric Christopher via llvm-commits]

echristo added a comment.

Couple of small comments, otherwise looks much better.

Thanks!

-eric


================
Comment at: lib/builtins/cpu_model.c:16-18
@@ +15,5 @@
+
+#if defined(i386) || defined(__i386__) || defined(__x86__) ||                  \
+    defined(_M_IX86) || defined(__x86_64__) || defined(_M_AMD64) ||            \
+    defined(_M_X64)
+
----------------
Can probably use a smaller set here maybe (and a couple of times later)?

Union of x86, x86_64, and windows provide is probably good enough.

================
Comment at: lib/builtins/cpu_model.c:126-147
@@ +125,24 @@
+
+// The check below fori386 was copied from clang's cpuid.h.
+static bool isCpuIdSupported() {
+#if defined(i386) || defined(__i386__)
+  int __cpuid_supported;
+  __asm("  pushfl\n"
+        "  popl   %%eax\n"
+        "  movl   %%eax,%%ecx\n"
+        "  xorl   $0x00200000,%%eax\n"
+        "  pushl  %%eax\n"
+        "  popfl\n"
+        "  pushfl\n"
+        "  popl   %%eax\n"
+        "  movl   $0,%0\n"
+        "  cmpl   %%eax,%%ecx\n"
+        "  je     1f\n"
+        "  movl   $1,%0\n"
+        "1:"
+        : "=r"(__cpuid_supported)
+        :
+        : "eax", "ecx");
+  if (!__cpuid_supported)
+    return 0;
+#endif
----------------
The comment and everything else feels weird here. Can you elaborate more? Also, the ifdefs seem unnecessary.


http://reviews.llvm.org/D22181