[3/5,AARCH64] Fix part num and implement dependency

On Tue, Nov 17, 2015 at 2:10 PM, Andrew Pinski <apinski@cavium.com> wrote:
>
> The way the current code was written assumes all cores have an unique part
> num which is not true.  What they have is an unique pair of implementer and
> part num.  This changes the code to look up that pair after the parsing
> of the two is done.
>
> Someone should test this on a big.little target too just to make sure
> the parsing is done correctly as I don't have access to one off hand.
>
> OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.


Here is an updated version of this patch after the other patches have
now gone in.
OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions
and -mcpu=native still works.

Thanks,
Andrew Pinski

>
> Thanks,
> Andrew Pinski
>
>
> * config/aarch64/driver-aarch64.c (host_detect_local_cpu):
> Rewrite handling of part num to handle the case where
> multiple implementers share the same part num.
> ---
>  gcc/config/aarch64/driver-aarch64.c | 46 ++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
> index 92388a9..ea1e856 100644
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -158,7 +158,7 @@ host_detect_local_cpu (int argc, const char **argv)
>    bool tune = false;
>    bool cpu = false;
>    unsigned int i = 0;
> -  unsigned int core_idx = 0;
> +  int core_idx = -1;
>    unsigned char imp = INVALID_IMP;
>    unsigned int cores[2] = { INVALID_CORE, INVALID_CORE };
>    unsigned int n_cores = 0;
> @@ -206,18 +206,13 @@ host_detect_local_cpu (int argc, const char **argv)
>        if (strstr (buf, "part") != NULL)
>         {
>           unsigned ccore = parse_field (buf);
> -         for (i = 0; cpu_data[i].name != NULL; i++)
> -           if (ccore == cpu_data[i].part_no
> -                && !contains_core_p (cores, ccore))
> -             {
> -                if (n_cores == 2)
> -                  goto not_found;
> -
> -                cores[n_cores++] = ccore;
> -               core_idx = i;
> -               arch_id = cpu_data[i].arch;
> -               break;
> -             }
> +         if (!contains_core_p (cores, ccore))
> +           {
> +              if (n_cores == 2)
> +                goto not_found;
> +
> +              cores[n_cores++] = ccore;
> +           }
>            continue;
>          }
>        if (!tune && !processed_exts && strstr (buf, "Features") != NULL)
> @@ -253,11 +248,19 @@ host_detect_local_cpu (int argc, const char **argv)
>    if (n_cores == 0 || n_cores > 2 || imp == INVALID_IMP)
>      goto not_found;
>
> -  if (arch && !arch_id)
> -    goto not_found;
> -
>    if (arch)
>      {
> +      /* Search for one of the cores in the list. */
> +      for (i = 0; cpu_data[i].name != NULL; i++)
> +       if (cpu_data[i].implementer_id == imp
> +           && contains_core_p (cores, cpu_data[i].part_no))
> +         {
> +           arch_id = cpu_data[i].arch;
> +           break;
> +         }
> +      if (!arch_id)
> +       goto not_found;
> +
>        const char* arch_name = get_arch_name_from_id (arch_id);
>
>        /* We got some arch indentifier that's not in aarch64-arches.def?  */
> @@ -284,8 +287,15 @@ host_detect_local_cpu (int argc, const char **argv)
>    /* The simple, non-big.LITTLE case.  */
>    else
>      {
> -      if (cpu_data[core_idx].implementer_id != imp)
> -        goto not_found;
> +      for (i = 0; cpu_data[i].name != NULL; i++)
> +       if (cores[0] == cpu_data[i].part_no
> +           && cpu_data[i].implementer_id == imp)
> +         {
> +           core_idx = i;
> +           break;
> +         }
> +      if (core_idx == -1)
> +       goto not_found;
>
>        res = concat ("-m", cpu ? "cpu" : "tune", "=",
>                        cpu_data[core_idx].name, NULL);
> --
> 1.9.1
>

On Sun, Oct 23, 2016 at 03:37:22PM -0700, Andrew Pinski wrote:
> On Tue, Nov 17, 2015 at 2:10 PM, Andrew Pinski <apinski@cavium.com> wrote:
> >
> > The way the current code was written assumes all cores have an unique part
> > num which is not true.  What they have is an unique pair of implementer and
> > part num.  This changes the code to look up that pair after the parsing
> > of the two is done.
> >
> > Someone should test this on a big.little target too just to make sure
> > the parsing is done correctly as I don't have access to one off hand.
> >
> > OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.
> 
> 
> Here is an updated version of this patch after the other patches have
> now gone in.
> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions
> and -mcpu=native still works.

OK.

The logic in this file looks a bit upside down in places. I might have a go
at refactoring it at some point in stage 3 or 4, but that's no reason to
block this patch for now.

Thanks,
James

> Index: config/aarch64/driver-aarch64.c
> ===================================================================
> --- config/aarch64/driver-aarch64.c	(revision 241437)
> +++ config/aarch64/driver-aarch64.c	(working copy)
> @@ -169,7 +169,6 @@ host_detect_local_cpu (int argc, const c
>    bool tune = false;
>    bool cpu = false;
>    unsigned int i = 0;
> -  unsigned int core_idx = 0;
>    unsigned char imp = INVALID_IMP;
>    unsigned int cores[2] = { INVALID_CORE, INVALID_CORE };
>    unsigned int n_cores = 0;
> @@ -219,18 +218,13 @@ host_detect_local_cpu (int argc, const c
>        if (strstr (buf, "part") != NULL)
>  	{
>  	  unsigned ccore = parse_field (buf);
> -	  for (i = 0; aarch64_cpu_data[i].name != NULL; i++)
> -	    if (ccore == aarch64_cpu_data[i].part_no
> -		&& !contains_core_p (cores, ccore))
> -	      {
> -		if (n_cores == 2)
> -		  goto not_found;
> -
> -		cores[n_cores++] = ccore;
> -		core_idx = i;
> -		arch_id = aarch64_cpu_data[i].arch;
> -		break;
> -	      }
> +	  if (!contains_core_p (cores, ccore))
> +	    {
> +	      if (n_cores == 2)
> +		goto not_found;
> +
> +	      cores[n_cores++] = ccore;
> +	    }
>  	  continue;
>  	}
>        if (!tune && !processed_exts && strstr (buf, "Features") != NULL)
> @@ -276,11 +270,19 @@ host_detect_local_cpu (int argc, const c
>    if (n_cores == 0 || n_cores > 2 || imp == INVALID_IMP)
>      goto not_found;
>  
> -  if (arch && !arch_id)
> -    goto not_found;
> -
>    if (arch)
>      {
> +      /* Search for one of the cores in the list. */
> +      for (i = 0; aarch64_cpu_data[i].name != NULL; i++)
> +	if (aarch64_cpu_data[i].implementer_id == imp
> +	    && contains_core_p (cores, aarch64_cpu_data[i].part_no))
> +	  {
> +	    arch_id = aarch64_cpu_data[i].arch;
> +	    break;
> +	  }
> +      if (!arch_id)
> +	goto not_found;
> +
>        struct aarch64_arch_driver_info* arch_info = get_arch_from_id (arch_id);
>  
>        /* We got some arch indentifier that's not in aarch64-arches.def?  */
> @@ -312,7 +314,15 @@ host_detect_local_cpu (int argc, const c
>    /* The simple, non-big.LITTLE case.  */
>    else
>      {
> -      if (aarch64_cpu_data[core_idx].implementer_id != imp)
> +      int core_idx = -1;
> +      for (i = 0; aarch64_cpu_data[i].name != NULL; i++)
> +	if (cores[0] == aarch64_cpu_data[i].part_no
> +	    && aarch64_cpu_data[i].implementer_id == imp)
> +	  {
> +	    core_idx = i;
> +	    break;
> +	  }
> +      if (core_idx == -1)
>  	goto not_found;
>  
>        res = concat ("-m", cpu ? "cpu" : "tune", "=",