[committed] OpenMP 4.0 affinity fixes and improvements (PR libgomp/58691)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

I've committed these changes to trunk.
The first two hunks hopefully fixes warnings (turned into -Werror) with
older glibc headers, the rest are changes requested in PR58691
- OMP_PROC_BIND default value is implementation defined, and Tobias
suggested a better implementation defined default - if OMP_PLACES
or GOMP_CPU_AFFINITY exist in environment and are successfully parsed,
then the default is omp_proc_bind_true, otherwise omp_proc_bind_false.
Explicit OMP_PROC_BIND=false makes those two env vars just parsed for
basic syntax errors, but doesn't actually build places lists from them.

2013-10-12  Jakub Jelinek  <jakub@redhat.com>

	PR libgomp/58691
	* config/linux/proc.c (gomp_cpuset_popcount): Add unused attribute
	to check variable.
	(gomp_init_num_threads): Move i variable declaration into
	#ifdef CPU_ALLOC_SIZE block.
	* config/linux/affinity.c (gomp_affinity_init_level): Test
	gomp_places_list_len == 0 rather than gomp_places_list == 0
	when checking for topology reading error.
	* team.c (gomp_team_start): Don't handle bind == omp_proc_bind_false.
	* env.c (parse_affinity): Add ignore argument, if true, don't populate
	gomp_places_list, only parse env var and always return false.
	(parse_places_var): Likewise.  Don't check gomp_global_icv.bind_var.
	(initialize_env): Always parse OMP_PLACES and GOMP_CPU_AFFINITY env
	vars, default to OMP_PROC_BIND=true if OMP_PROC_BIND wasn't specified
	and either of these variables were parsed correctly into a places
	list.

--- libgomp/config/linux/proc.c.jj	2013-10-11 11:23:59.000000000 +0200
+++ libgomp/config/linux/proc.c	2013-10-11 23:17:18.533108120 +0200
@@ -59,7 +59,7 @@ gomp_cpuset_popcount (unsigned long cpus
   size_t i;
   unsigned long ret = 0;
   extern int check[sizeof (cpusetp->__bits[0]) == sizeof (unsigned long int)
-		   ? 1 : -1];
+		   ? 1 : -1] __attribute__((unused));
 
   for (i = 0; i < cpusetsize / sizeof (cpusetp->__bits[0]); i++)
     {
@@ -94,7 +94,6 @@ gomp_init_num_threads (void)
 					gomp_cpusetp);
       if (ret == 0)
 	{
-	  unsigned long i;
 	  /* Count only the CPUs this process can use.  */
 	  gomp_global_icv.nthreads_var
 	    = gomp_cpuset_popcount (gomp_cpuset_size, gomp_cpusetp);
@@ -102,6 +101,7 @@ gomp_init_num_threads (void)
 	    break;
 	  gomp_get_cpuset_size = gomp_cpuset_size;
 #ifdef CPU_ALLOC_SIZE
+	  unsigned long i;
 	  for (i = gomp_cpuset_size * 8; i; i--)
 	    if (CPU_ISSET_S (i - 1, gomp_cpuset_size, gomp_cpusetp))
 	      break;
--- libgomp/config/linux/affinity.c.jj	2013-10-11 11:23:59.000000000 +0200
+++ libgomp/config/linux/affinity.c	2013-10-11 23:35:05.566620759 +0200
@@ -309,7 +309,7 @@ gomp_affinity_init_level (int level, uns
 		fclose (f);
 	      }
 	  }
-      if (gomp_places_list == 0)
+      if (gomp_places_list_len == 0)
 	{
 	  if (!quiet)
 	    gomp_error ("Error reading %s topology",
--- libgomp/team.c.jj	2013-10-11 11:23:59.000000000 +0200
+++ libgomp/team.c	2013-10-11 23:27:52.928843235 +0200
@@ -339,8 +339,6 @@ gomp_team_start (void (*fn) (void *), vo
 
   if (__builtin_expect (gomp_places_list != NULL, 0))
     {
-      if (bind == omp_proc_bind_false)
-	bind = omp_proc_bind_true;
       /* Depending on chosen proc_bind model, set subpartition
 	 for the master thread and initialize helper variables
 	 P and optionally S, K and/or REST used by later place
@@ -348,9 +346,6 @@ gomp_team_start (void (*fn) (void *), vo
       p = thr->place - 1;
       switch (bind)
 	{
-	case omp_proc_bind_false:
-	  bind = omp_proc_bind_true;
-	  /* FALLTHRU */
 	case omp_proc_bind_true:
 	case omp_proc_bind_close:
 	  if (nthreads > thr->ts.place_partition_len)
--- libgomp/env.c.jj	2013-10-11 11:23:59.000000000 +0200
+++ libgomp/env.c	2013-10-12 00:10:02.670107433 +0200
@@ -548,7 +548,7 @@ parse_one_place (char **envp, bool *nega
 }
 
 static bool
-parse_places_var (const char *name)
+parse_places_var (const char *name, bool ignore)
 {
   char *env = getenv (name), *end;
   bool any_negate = false;
@@ -604,6 +604,10 @@ parse_places_var (const char *name)
 	  if (*env != '\0')
 	    goto invalid;
 	}
+
+      if (ignore)
+	return false;
+
       return gomp_affinity_init_level (level, count, false);
     }
 
@@ -634,7 +638,7 @@ parse_places_var (const char *name)
     }
   while (1);
 
-  if (gomp_global_icv.bind_var == omp_proc_bind_false)
+  if (ignore)
     return false;
 
   gomp_places_list_len = 0;
@@ -911,7 +915,7 @@ parse_wait_policy (void)
    present and it was successfully parsed.  */
 
 static bool
-parse_affinity (void)
+parse_affinity (bool ignore)
 {
   char *env, *end, *start;
   int pass;
@@ -928,6 +932,9 @@ parse_affinity (void)
       env = start;
       if (pass == 1)
 	{
+	  if (ignore)
+	    return false;
+
 	  gomp_places_list_len = 0;
 	  gomp_places_list = gomp_affinity_alloc (count, true);
 	  if (gomp_places_list == NULL)
@@ -995,6 +1002,7 @@ parse_affinity (void)
     {
       free (gomp_places_list);
       gomp_places_list = NULL;
+      return false;
     }
   return true;
 
@@ -1183,14 +1191,34 @@ initialize_env (void)
 				 &gomp_nthreads_var_list,
 				 &gomp_nthreads_var_list_len))
     gomp_global_icv.nthreads_var = gomp_available_cpus;
-  if (!parse_bind_var ("OMP_PROC_BIND",
-		       &gomp_global_icv.bind_var,
-		       &gomp_bind_var_list,
-		       &gomp_bind_var_list_len))
-    gomp_global_icv.bind_var = omp_proc_bind_false;
-  if (parse_places_var ("OMP_PLACES")
-      || parse_affinity ()
-      || gomp_global_icv.bind_var)
+  bool ignore = false;
+  if (parse_bind_var ("OMP_PROC_BIND",
+		      &gomp_global_icv.bind_var,
+		      &gomp_bind_var_list,
+		      &gomp_bind_var_list_len)
+      && gomp_global_icv.bind_var == omp_proc_bind_false)
+    ignore = true;
+  /* Make sure OMP_PLACES and GOMP_CPU_AFFINITY env vars are always
+     parsed if present in the environment.  If OMP_PROC_BIND was set
+     explictly to false, don't populate places list though.  If places
+     list was successfully set from OMP_PLACES, only parse but don't process
+     GOMP_CPU_AFFINITY.  If OMP_PROC_BIND was not set in the environment,
+     default to OMP_PROC_BIND=true if OMP_PLACES or GOMP_CPU_AFFINITY
+     was successfully parsed into a places list, otherwise to
+     OMP_PROC_BIND=false.  */
+  if (parse_places_var ("OMP_PLACES", ignore))
+    {
+      if (gomp_global_icv.bind_var == omp_proc_bind_false)
+	gomp_global_icv.bind_var = true;
+      ignore = true;
+    }
+  if (parse_affinity (ignore))
+    {
+      if (gomp_global_icv.bind_var == omp_proc_bind_false)
+	gomp_global_icv.bind_var = true;
+      ignore = true;
+    }
+  if (gomp_global_icv.bind_var != omp_proc_bind_false)
     gomp_init_affinity ();
   wait_policy = parse_wait_policy ();
   if (!parse_spincount ("GOMP_SPINCOUNT", &gomp_spin_count_var))

	Jakub