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

[Eric Christopher via llvm-commits]

echristo added a comment.

Couple more inline comments, otherwise it's looking pretty good to me.

Thanks!

-eric


================
Comment at: lib/builtins/cpu_model.c:130
@@ +129,3 @@
+static bool isCpuIdSupported() {
+//FIXME: Is this valid for MSVC?
+#if defined(__GNUC__) || defined(__clang__)
----------------
The syntax isn't for sure. :)

MSDN doesn't conditionalize the __cpuid intrinsic so I don't think this routine is necessary for MSVC and the return 1 is fine. Also, return true/false?

================
Comment at: lib/builtins/cpu_model.c:202
@@ +201,3 @@
+/// the 4 values in the specified arguments.  If we can't run cpuid on the host,
+/// return true.
+static bool getX86CpuIDAndInfoEx(unsigned value, unsigned subleaf,
----------------
The return values here seem duplicated with the isCpuIDSupported routine?

================
Comment at: lib/builtins/cpu_model.c:263
@@ +262,3 @@
+  __asm__(".byte 0x0f, 0x01, 0xd0" : "=a"(*rEAX), "=d"(*rEDX) : "c"(0));
+  return false;
+#elif defined(_MSC_FULL_VER) && defined(_XCR_XFEATURE_ENABLED_MASK)
----------------
Go ahead and document the return value and the arguments here please.


http://reviews.llvm.org/D22181