This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, i386]: Add support for CPUID 4 in driver-i386.c




Sent from my iPhone

On Oct 11, 2008, at 8:31 PM, Andi Kleen <andi@firstfloor.org> wrote:

n Sat, Oct 11, 2008 at 07:56:23PM +0200, Uros Bizjak wrote:
Hello!

This patch is loosely based on Andi's patch to use CPUID function 4 to
describe Intel caches. In addition to parsing CPUID function 4 values,
attached patch updates CPUID 2 table to the latest known CPUID
documentation, it fixes Xeon MP CPUID fn 2 problems and handles number
of times to execute CPUID fn 2 instruction to obtain complete cache
hierarchy description.

I suspect the CPUID 2 table update is not really needed, because most CPUs now can describe themselves using CPUID 4.

The xeon_mp variable is quite misnamed because there are lots of quite
different SKUs calling themselves that. Also it wouldn't surprise
me if that CPU already had CPUID 4, so the hack is likely not needed.

But the big open question is what to do with level2 vs level3?
Perhaps pass both as arguments and let the optimization passes decide?

+#define __cpuid_count(level, count, a, b, c, d)        \
+  __asm__ ("xchgl\t%%ebx, %1\n\t"            \
+       "cpuid\n\t"                    \
+       "xchgl\t%%ebx, %1\n\t"            \
+       : "=a" (a), "=r" (b), "=c" (c), "=d" (d)    \
+       : "0" (level), "2" (count))

I don't think a PIC cpuid_count is really needed.

Why? X86 Darwin for an example is PIC by default. So this needed to be able to compile it there.




+  for (i = 24; i >= 0; i -= 8)
+    switch ((reg >> i) & 0xff)
+      {
+      case 0x0a:

As a personal nit if you're going to redo this anyways then a lookup table would be much shorter than code.

+    case CACHE_END:
+      return;
+    case CACHE_DATA:
+    case CACHE_UNIFIED:
+      {
+        switch ((eax >> 5) & 0x07)
+          {
+          case 1:
+        cache = level1;
+        break;
+          case 2:
+        cache = level2;
+        break;
+          default:
+        cache = NULL;

That seems like a weird value for a enum


-Andi
--
ak@linux.intel.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]