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

[Alina Sbirlea via llvm-commits]

asbirlea added inline comments.

================
Comment at: lib/builtins/cpu_model.c:729
@@ +728,3 @@
+
+  /* Assume cpuid insn present. Run in level 0 to get vendor id. */
+  if (getX86CpuIDAndInfo(0, &MaxLeaf, &Vendor, &ECX, &EDX)) {
----------------
joerg wrote:
> asbirlea wrote:
> > echristo wrote:
> > > joerg wrote:
> > > > This assumption will bit us, BTW. We've bug reports for OpenSSL crashing not too long ago on CPUs without CPUID support. At least for i386, presence definitely must be checked first via the corresponding eflags bit.
> > > Yeesh. You should probably update the code in Host.cpp then :)
> > I'm not familiar with what flags need to be checked. Could you elaborate or update Host.cpp to resolve this? 
> Take a look at clang's cpuid.h and __get_cpuid_max. I wonder if we can/should assume the presence of that header. What's the Windows situation here?
Is this roughly what you had in mind?

// insert @:730; also include cpuid.h
+  MaxLeaf = __get_cpuid_max(0, &Vendor);
+  if(!MaxLeaf)
+    return -1;

Alternatively just copy the check ifdefed by i386?
Both options will work from preliminary builds.

The plan was to support Windows as well. I'm not clear if this will still be the case.


http://reviews.llvm.org/D21033