diff mbox

[i386] Handle extended family cpuid info for AMD

Message ID EB4625145972F94C9680D8CADD651615787D62B5@SATLEXDAG02.amd.com
State New
Headers show

Commit Message

Gopalasubramanian, Ganesh Aug. 1, 2014, 2:52 p.m. UTC 
> In this case, having only check for family ID should be enough. If

>BTVER1 and BTVER2 can be uniquely determined by their family IDs ,


>IMO, this would be the most future-proof approach. Signature checks will override family id checks which will override cpuid checks.


Thank you Uros!

I have modified source only for BTVER2. 
The way BTVER1 is currently assigned to processor includes more than one family. So, I am leaving that unmoved.

Bootstrap passes.
Is it OK for trunk and backport to open branches.

Regards
-Ganesh

Comments

Jakub Jelinek Aug. 1, 2014, 3 p.m. UTC | #1 
On Fri, Aug 01, 2014 at 02:52:28PM +0000, Gopalasubramanian, Ganesh wrote:
> --- a/gcc/config/i386/driver-i386.c
> +++ b/gcc/config/i386/driver-i386.c
> @@ -432,7 +432,8 @@ const char *host_detect_local_cpu (int argc, const char **argv)
>  
>    model = (eax >> 4) & 0x0f;
>    family = (eax >> 8) & 0x0f;
> -  if (vendor == signature_INTEL_ebx)
> +  if ((vendor == signature_INTEL_ebx) ||
> +      (vendor == signature_AMD_ebx))

Wrong formatting.  No ()s around the comparisons needed, and
|| should go on the second line, not first.

> @@ -576,7 +577,7 @@ const char *host_detect_local_cpu (int argc, const char **argv)
>  
>        if (name == signature_NSC_ebx)
>  	processor = PROCESSOR_GEODE;
> -      else if (has_movbe)
> +      else if (family == 22)
>  	processor = PROCESSOR_BTVER2;

Wouldn't it be safer to use has_movbe && family == 22?
I mean, especially with emulators which choose to provide some architecture,
but disable some CPUID flags it is IMHO safer to also check the flags.

	Jakub
Uros Bizjak Aug. 1, 2014, 6:25 p.m. UTC | #2 
On Fri, Aug 1, 2014 at 5:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> --- a/gcc/config/i386/driver-i386.c
>> +++ b/gcc/config/i386/driver-i386.c
>> @@ -432,7 +432,8 @@ const char *host_detect_local_cpu (int argc, const char **argv)
>>
>>    model = (eax >> 4) & 0x0f;
>>    family = (eax >> 8) & 0x0f;
>> -  if (vendor == signature_INTEL_ebx)
>> +  if ((vendor == signature_INTEL_ebx) ||
>> +      (vendor == signature_AMD_ebx))
>
> Wrong formatting.  No ()s around the comparisons needed, and
> || should go on the second line, not first.
>
>> @@ -576,7 +577,7 @@ const char *host_detect_local_cpu (int argc, const char **argv)
>>
>>        if (name == signature_NSC_ebx)
>>       processor = PROCESSOR_GEODE;
>> -      else if (has_movbe)
>> +      else if (family == 22)
>>       processor = PROCESSOR_BTVER2;
>
> Wouldn't it be safer to use has_movbe && family == 22?
> I mean, especially with emulators which choose to provide some architecture,
> but disable some CPUID flags it is IMHO safer to also check the flags.

OK, after rethinking this, let's go with the safer original patch
(sorry for a bit of confusion, but we cleared a lot of open questions
during the discussion).

The original patch is OK for mainline and branches.

OTOH, it looks we will have to redesign -march=native someday to only
pass options, derived from cpuid and eventually pass -mtune based on
detected family.

Thanks,
Uros.
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 706fedc..202bd99 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@ 
+2014-08-01  Ganesh Gopalasubramanian  <Ganesh.Gopalasubramanian@amd.com>
+
+	* config/i386/driver-i386.c (host_detect_local_cpu): Handle AMD's extended family
+	information. Handle BTVER2 cpu with cpuid family value.  
+
 2014-07-31  James Greenhalgh  <james.greenhalgh@arm.com>
 
 	* config/aarch64/arm_neon.h (vpadd_<suf><8,16,32,64>): Move to
diff --git a/gcc/config/i386/driver-i386.c b/gcc/config/i386/driver-i386.c
index 1c6385f..0402c90 100644
--- a/gcc/config/i386/driver-i386.c
+++ b/gcc/config/i386/driver-i386.c
@@ -432,7 +432,8 @@  const char *host_detect_local_cpu (int argc, const char **argv)
 
   model = (eax >> 4) & 0x0f;
   family = (eax >> 8) & 0x0f;
-  if (vendor == signature_INTEL_ebx)
+  if ((vendor == signature_INTEL_ebx) ||
+      (vendor == signature_AMD_ebx))
     {
       unsigned int extended_model, extended_family;
 
@@ -576,7 +577,7 @@  const char *host_detect_local_cpu (int argc, const char **argv)
 
       if (name == signature_NSC_ebx)
 	processor = PROCESSOR_GEODE;
-      else if (has_movbe)
+      else if (family == 22)
 	processor = PROCESSOR_BTVER2;
       else if (has_avx2)
         processor = PROCESSOR_BDVER4;