Added sockets to the syntax of KMP_PLACE_THREADS environment variable.

[jlpeyton]

Summary

Added (optional) sockets to the syntax of the KMP_PLACE_THREADS environment variable.
Some limitations:

• the number of sockets and then optional offset should be specified first (before other parameters).
• the letter designation is mandatory for sockets and then for other parameters.
• if number of cores is specified first, then the number of sockets is defaulted to all sockets on the machine; also, the old syntax is partially supported if sockets are skipped.
• if number of threads per core is specified first, then the number of sockets and cores per socket are defaulted to all sockets and all cores per socket respectively.
• the number of cores per socket cannot be specified before sockets or after threads per core.
• the number of threads per core can be specified before or after core-offset (old syntax required it to be before core-offset);
• parameters delimiter can be: empty, comma, lower-case x;
• spaces are allowed around numbers, around letters, around delimiter.

Approximate shorthand specification:
KMP_PLACE_THREADS="[num_sockets(S|s)[[delim]offset(O|o)][delim]][num_cores_per_socket(C|c)[[delim]offset(O|o)][delim]][num_threads_per_core(T|t)]"

[jcownie]

Can't we roll this up into something which is more table-driven (or loopy) ? (Something more like the code in the testbed runtime, which, admittedly, doesn't parse this yet, but clearly could quite easily.
Note that there's a loop here that extracts the sub-components of the string, then another loop to process them, rather than having five blocks of code, much of which is replicated.

static void parsePlaceThreads(char const * text, int * sockets, int * coresPerSocket, int *threadsPerCore)
{
    // Default is (quoting Manuel) "I know nothing". (Vide Fawlty Towers...)
    *sockets = *coresPerSocket = *threadsPerCore = 0;

    if (!text)
        return;

    // We have something...
    // Split the input into strings at 'x','X' or ','
    std::string components[unknown_e];
    std::string value = text;

    size_t base = 0;
    for (int i=0; i<unknown_e; i++)
    {
        int sepPos = value.find('x',base);
        if (sepPos == std::string::npos)
        {
            sepPos = value.find('X',base);
            if (sepPos == std::string::npos)
                sepPos = value.find (',', base);
        }
        if (sepPos == std::string::npos)
        {
            components[i] = std::string (value, base, value.length());
            break;
        }
        else
        {
            size_t len = sepPos - base;
            components[i] = std::string (value, base, len);
        }
        base = sepPos+1;
    }

    // Actually we found nothing
    if (components[0].length() == 0)
        return;

    bool seen[unknown_e];
    for (int i=0; i<unknown_e; i++)
        seen[i] = false;

    // Check each component
    for (int i=0; i<unknown_e; i++)
    {
        if (components[i].length() == 0)
            break;

        componentType_e what = getType (components[i]);
        if (what == unknown_e)
        {
            what = componentType_e(i);
            if (seen[what])
                what = componentType_e(((int)what)+1);
        }
        if (seen[what] || what == unknown_e)
        {
            fatalError ("Cannot parse KMP_PLACE_THREADS (duplicate field): %s.", text);
        }
        else
        {
            seen[what] = true;
        }
        char const * start = components[i].c_str();
        char * end   = 0;
        long int value = strtol(start, &end, 0);

        if (end == start)
        {
            fatalError ("Cannot parse KMP_PLACE_THREADS (failed to parse a number): %s.", text);
        }
        if (*end == '@')
        {
            fatalError("Offsets in KMP_PLACE_THREADS are not currently supported: %s.", text);
        }

        switch (what)
        {
            case sockets_e: *sockets = value;        break;
            case cores_e:   *coresPerSocket = value; break;
            case threads_e: *threadsPerCore = value; break;
        }
    }
}

[AndreyChurbanov]

LGTM

Jim,

We can think of your suggestion as a separate code improvement, given that we make syntax of the KMP_PLACE_THREADS stricter (mandatory delimiters, strict order of elements).

Currently delimiters are optional, and some freedom in order of elements allowed to support initial socket-less syntax. That makes the structured loopy parsing problematic.

openmp/trunk/runtime/src/i18n/en_US.txt

 AffGranCantUseMultGroups     "Cannot use affinity granularity \"%1$s\" with multiple Windows* OS processor groups, using \"%2$s\"."
 AffWindowsProcGroupMap       "%1$s: Mapping Windows* OS processor group <i> proc <j> to OS proc 64*<i>+<j>."
 AffOSProcToGroup             "%1$s: OS proc %2$d maps to Windows* OS processor group %3$d proc %4$d"
 AffBalancedNotAvail          "%1$s: Affinity balanced is not available."
 OBSOLETE                     "%1$s: granularity=core will be used."
 EnvLockWarn                  "%1$s must be set prior to first OMP lock call or critical section; ignored."
 FutexNotSupported            "futex system call not supported; %1$s=%2$s ignored."
 AffGranUsing                 "%1$s: granularity=%2$s will be used."
-AffThrPlaceInvalid           "%1$s: invalid value \"%2$s\", valid format is \"nC,mT[,kO]\"."
+AffThrPlaceInvalid           "%1$s: invalid value \"%2$s\", valid format is \"nS[,nO],nC[,nO],nT "
+                             "(nSockets@offset, nCores@offset, nTthreads per core)\"."
 AffThrPlaceUnsupported       "KMP_PLACE_THREADS ignored: unsupported architecture."
 AffThrPlaceManyCores         "KMP_PLACE_THREADS ignored: too many cores requested."
 SyntaxErrorUsing             "%1$s: syntax error, using %2$s."
 AdaptiveNotSupported         "%1$s: Adaptive locks are not supported; using queuing."
 EnvSyntaxError               "%1$s: Invalid symbols found. Check the value \"%2$s\"."
 EnvSpacesNotAllowed          "%1$s: Spaces between digits are not allowed \"%2$s\"."
 BoundToOSProcSet             "%1$s: pid %2$d thread %3$d bound to OS proc set %4$s"
 CnsLoopIncrIllegal           "%1$s error: parallel loop increment and condition are inconsistent."
 NoGompCancellation           "libgomp cancellation is not currently supported."
 AffThrPlaceNonUniform        "KMP_PLACE_THREADS ignored: non-uniform topology."
 AffThrPlaceNonThreeLevel     "KMP_PLACE_THREADS ignored: only three-level topology is supported."
 AffGranTopGroup              "%1$s: granularity=%2$s is not supported with KMP_TOPOLOGY_METHOD=group. Using \"granularity=fine\"."
 AffGranGroupType             "%1$s: granularity=group is not supported with KMP_AFFINITY=%2$s. Using \"granularity=core\"."
+AffThrPlaceManySockets       "KMP_PLACE_THREADS ignored: too many sockets requested."
 
 
 # --------------------------------------------------------------------------------------------------
 -*- HINTS -*-
 # --------------------------------------------------------------------------------------------------
 
 # Hints. Hint may be printed after a message. Usually it is longer explanation text or suggestion.
 # To maintain hint numbers (they are visible to customers), add new hints to the end.

openmp/trunk/runtime/src/kmp.h

typedef enum kmp_cancel_kind_t {
     cancel_noreq = 0,
     cancel_parallel = 1,
     cancel_loop = 2,
     cancel_sections = 3,
     cancel_taskgroup = 4
 } kmp_cancel_kind_t;
 #endif // OMP_40_ENABLED
 
+extern int __kmp_place_num_sockets;
+extern int __kmp_place_socket_offset;
 extern int __kmp_place_num_cores;
-extern int __kmp_place_num_threads_per_core;
 extern int __kmp_place_core_offset;
+extern int __kmp_place_num_threads_per_core;
 
 /* ------------------------------------------------------------------------ */
 /* ------------------------------------------------------------------------ */
 
 #define KMP_PAD(type, sz)     (sizeof(type) + (sz - ((sizeof(type) - 1) % (sz)) - 1))
 
 //
 // We need to avoid using -1 as a GTID as +1 is added to the gtid
__kmp_determine_reduction_method( ident_t *loc, kmp_int32 global_tid,
                                   kmp_critical_name *lck );
 
 // this function is for testing set/get/determine reduce method
 KMP_EXPORT kmp_int32 __kmp_get_reduce_method( void );
 
 KMP_EXPORT kmp_uint64 __kmpc_get_taskid();
 KMP_EXPORT kmp_uint64 __kmpc_get_parent_taskid();
 
-KMP_EXPORT void __kmpc_place_threads(int,int,int);
+// this function exported for testing of KMP_PLACE_THREADS functionality
+KMP_EXPORT void __kmpc_place_threads(int,int,int,int,int);
 
 /* ------------------------------------------------------------------------ */
 /* ------------------------------------------------------------------------ */
 
 // C++ port
 // missing 'extern "C"' declarations
 
 KMP_EXPORT kmp_int32 __kmpc_in_parallel( ident_t *loc );

openmp/trunk/runtime/src/kmp_affinity.cpp

 # endif /* OMP_40_ENABLED */
 
 #undef ADD_MASK
 #undef ADD_MASK_OSID
 
 static void
 __kmp_apply_thread_places(AddrUnsPair **pAddr, int depth)
 {
-    if ( __kmp_place_num_cores == 0 ) {
-        if ( __kmp_place_num_threads_per_core == 0 ) {
-            return;   // no cores limiting actions requested, exit
-        }
+    if (__kmp_place_num_sockets == 0 &&
+        __kmp_place_num_cores == 0 &&
+        __kmp_place_num_threads_per_core == 0 )
+        return;   // no topology limiting actions requested, exit
+    if (__kmp_place_num_sockets == 0)
+        __kmp_place_num_sockets = nPackages;    // use all available sockets
+    if (__kmp_place_num_cores == 0)
         __kmp_place_num_cores = nCoresPerPkg;   // use all available cores
-    }
+    if (__kmp_place_num_threads_per_core == 0 ||
+        __kmp_place_num_threads_per_core > __kmp_nThreadsPerCore)
+        __kmp_place_num_threads_per_core = __kmp_nThreadsPerCore; // use all HW contexts
+
     if ( !__kmp_affinity_uniform_topology() ) {
         KMP_WARNING( AffThrPlaceNonUniform );
         return; // don't support non-uniform topology
     }
     if ( depth != 3 ) {
         KMP_WARNING( AffThrPlaceNonThreeLevel );
         return; // don't support not-3-level topology
     }
-    if ( __kmp_place_num_threads_per_core == 0 ) {
-        __kmp_place_num_threads_per_core = __kmp_nThreadsPerCore;  // use all HW contexts
+    if (__kmp_place_socket_offset + __kmp_place_num_sockets > nPackages) {
+        KMP_WARNING(AffThrPlaceManySockets);
+        return;
     }
     if ( __kmp_place_core_offset + __kmp_place_num_cores > nCoresPerPkg ) {
         KMP_WARNING( AffThrPlaceManyCores );
         return;
     }
 
     AddrUnsPair *newAddr = (AddrUnsPair *)__kmp_allocate( sizeof(AddrUnsPair) *
-                            nPackages * __kmp_place_num_cores * __kmp_place_num_threads_per_core);
+        __kmp_place_num_sockets * __kmp_place_num_cores * __kmp_place_num_threads_per_core);
+
     int i, j, k, n_old = 0, n_new = 0;
-    for ( i = 0; i < nPackages; ++i ) {
-        for ( j = 0; j < nCoresPerPkg; ++j ) {
-            if ( j < __kmp_place_core_offset || j >= __kmp_place_core_offset + __kmp_place_num_cores ) {
-                n_old += __kmp_nThreadsPerCore;   // skip not-requested core
-            } else {
-                for ( k = 0; k < __kmp_nThreadsPerCore; ++k ) {
-                    if ( k < __kmp_place_num_threads_per_core ) {
-                        newAddr[n_new] = (*pAddr)[n_old];   // copy requested core' data to new location
+    for (i = 0; i < nPackages; ++i)
+        if (i < __kmp_place_socket_offset ||
+            i >= __kmp_place_socket_offset + __kmp_place_num_sockets)
+            n_old += nCoresPerPkg * __kmp_nThreadsPerCore; // skip not-requested socket
+        else
+            for (j = 0; j < nCoresPerPkg; ++j) // walk through requested socket
+                if (j < __kmp_place_core_offset ||
+                    j >= __kmp_place_core_offset + __kmp_place_num_cores)
+                    n_old += __kmp_nThreadsPerCore; // skip not-requested core
+                else
+                    for (k = 0; k < __kmp_nThreadsPerCore; ++k) { // walk through requested core
+                        if (k < __kmp_place_num_threads_per_core) {
+                            newAddr[n_new] = (*pAddr)[n_old]; // collect requested thread's data
                             n_new++;
                         }
                         n_old++;
                     }
-            }
-        }
-    }
+    KMP_DEBUG_ASSERT(n_old == nPackages * nCoresPerPkg * __kmp_nThreadsPerCore);
+    KMP_DEBUG_ASSERT(n_new == __kmp_place_num_sockets * __kmp_place_num_cores *
+                     __kmp_place_num_threads_per_core);
+
+    nPackages = __kmp_place_num_sockets;                      // correct nPackages
     nCoresPerPkg = __kmp_place_num_cores;                     // correct nCoresPerPkg
     __kmp_nThreadsPerCore = __kmp_place_num_threads_per_core; // correct __kmp_nThreadsPerCore
     __kmp_avail_proc = n_new;                                 // correct avail_proc
     __kmp_ncores = nPackages * __kmp_place_num_cores;         // correct ncores
 
     __kmp_free( *pAddr );
     *pAddr = newAddr;      // replace old topology with new one
 }

openmp/trunk/runtime/src/kmp_csupport.c

if ( gtid < 0 ) {
         return 0;
     }; // if
     thread      = __kmp_thread_from_gtid( gtid );
     parent_task = thread->th.th_current_task->td_parent;
     return ( parent_task == NULL ? 0 : parent_task->td_task_id );
 
 } // __kmpc_get_parent_taskid
 
-void __kmpc_place_threads(int nC, int nT, int nO)
+void __kmpc_place_threads(int nS, int sO, int nC, int cO, int nT)
 {
     if ( ! __kmp_init_serial ) {
         __kmp_serial_initialize();
     }
+    __kmp_place_num_sockets = nS;
+    __kmp_place_socket_offset = sO;
     __kmp_place_num_cores = nC;
+    __kmp_place_core_offset = cO;
     __kmp_place_num_threads_per_core = nT;
-    __kmp_place_core_offset = nO;
 }
 
 // end of file //
 

openmp/trunk/runtime/src/kmp_global.c

 
 #endif /* KMP_AFFINITY_SUPPORTED */
 
 #if OMP_40_ENABLED
 kmp_nested_proc_bind_t __kmp_nested_proc_bind = { NULL, 0, 0 };
 int __kmp_affinity_num_places = 0;
 #endif
 
+int __kmp_place_num_sockets = 0;
+int __kmp_place_socket_offset = 0;
 int __kmp_place_num_cores = 0;
-int __kmp_place_num_threads_per_core = 0;
 int __kmp_place_core_offset = 0;
+int __kmp_place_num_threads_per_core = 0;
 
 kmp_tasking_mode_t __kmp_tasking_mode = tskm_task_teams;
 
 /* This check ensures that the compiler is passing the correct data type
  * for the flags formal parameter of the function kmpc_omp_task_alloc().
  * If the type is not a 4-byte type, then give an error message about
  * a non-positive length array pointing here.  If that happens, the
  * kmp_tasking_flags_t structure must be redefined to have exactly 32 bits.

openmp/trunk/runtime/src/kmp_settings.c

 // -------------------------------------------------------------------------------------------------
 // KMP_PLACE_THREADS
 // -------------------------------------------------------------------------------------------------
 
 static void
 __kmp_stg_parse_place_threads( char const * name, char const * value, void * data ) {
     // Value example: 5Cx2Tx15O
     // Which means "use 5 cores with offset 15, 2 threads per core"
-
+    // AC: extended to sockets level:
+    //     2s,6o,2c,2o,2t   or   2s,6o,2c,2t,2o
+    //     (to not break legacy code core-offset can be last).
+    // Note: not all syntax errors are analyzed, some may be skipped.
+#define CHECK_DELIM(_x)   (*(_x) == ',' || *(_x) == '@' || *(_x) == 'x')
     int         num;
-    int         prev_delim = 0;
+    int         flagS = 0, flagC = 0, flagT = 0;
     const char *next = value;
     const char *prev;
 
-    SKIP_WS( next );
-    if ( *next == '\0' ) {
-        return;   // leave default values
-    }
-
-    // Get num_cores first
-    if ( *next >= '0' && *next <= '9' ) {
+    SKIP_WS(next);  // skip white spaces
+    if (*next == '\0')
+        return;   // no data provided, retain default values
+    // Get num_sockets first (or whatever specified)
+    if (*next >= '0' && *next <= '9') {
         prev = next;
-        SKIP_DIGITS( next );
-        num = __kmp_str_to_int( prev, *next );
-        SKIP_WS( next );
-        if ( *next == 'C' || *next == 'c' ) {
-            __kmp_place_num_cores = num;
+        SKIP_DIGITS(next);
+        num = __kmp_str_to_int(prev, *next);
+        SKIP_WS(next);
+        if (*next == 's' || *next == 'S') {
+            __kmp_place_num_sockets = num;
+            flagS = 1; // got num sockets
             next++;
-        } else if ( *next == ',' || *next == 'x' ) {
+        } else if (*next == 'c' || *next == 'C') {
             __kmp_place_num_cores = num;
-            prev_delim = 1;
+            flagS = flagC = 1; // sockets were not specified - use default
+            next++;
+        } else if (CHECK_DELIM(next)) {
+            __kmp_place_num_cores = num; // no letter-designator - num cores
+            flagS = flagC = 1; // sockets were not specified - use default
             next++;
-        } else if ( *next == 'T' || *next == 't' ) {
+        } else if (*next == 't' || *next == 'T') {
             __kmp_place_num_threads_per_core = num;
+            // sockets, cores were not specified - use default
             return;   // we ignore offset value in case all cores are used
-        } else if ( *next == '\0' ) {
+        } else if (*next == '\0') {
             __kmp_place_num_cores = num;
-            return;   // the only value provided
+            return;   // the only value provided - set num cores
         } else {
-            KMP_WARNING( AffThrPlaceInvalid, name, value );
+            KMP_WARNING(AffThrPlaceInvalid, name, value);
             return;
         }
-    } else if ( *next == ',' || *next == 'x' ) {
-        // First character is delimiter, skip it, leave num_cores default value
-        prev_delim = 2;
-        next++;
     } else {
-        KMP_WARNING( AffThrPlaceInvalid, name, value );
+        KMP_WARNING(AffThrPlaceInvalid, name, value);
         return;
     }
-    SKIP_WS( next );
-    if ( *next == '\0' ) {
+    KMP_DEBUG_ASSERT(flagS); // num sockets should already be set here
+    SKIP_WS(next);
+    if (*next == '\0')
         return;   // " n  " - something like this
-    }
-    if ( ( *next == ',' || *next == 'x' ) && !prev_delim ) {
-        prev_delim = 1;
-        next++;   // skip delimiter after num_core value
-        SKIP_WS( next );
+    if (CHECK_DELIM(next)) {
+        next++;   // skip delimiter
+        SKIP_WS(next);
     }
 
-    // Get threads_per_core next
-    if ( *next >= '0' && *next <= '9' ) {
-        prev_delim = 0;
+    // Get second value (could be offset, num_cores, num_threads)
+    if (*next >= '0' && *next <= '9') {
         prev = next;
-        SKIP_DIGITS( next );
-        num = __kmp_str_to_int( prev, *next );
-        SKIP_WS( next );
-        if ( *next == 'T' || *next == 't' ) {
-            __kmp_place_num_threads_per_core = num;
+        SKIP_DIGITS(next);
+        num = __kmp_str_to_int(prev, *next);
+        SKIP_WS(next);
+        if (*next == 'o' || *next == 'O') { // offset specified
+            if (flagC) { // whether num_cores already specified (when sockets skipped)
+                __kmp_place_core_offset = num;
+            } else {
+                __kmp_place_socket_offset = num;
+            }
             next++;
-        } else if ( *next == ',' || *next == 'x' ) {
-            __kmp_place_num_threads_per_core = num;
-            prev_delim = 1;
+        } else if (*next == 'c' || *next == 'C') {
+            KMP_DEBUG_ASSERT(flagC == 0);
+            __kmp_place_num_cores = num;
+            flagC = 1;
             next++;
-        } else if ( *next == 'O' || *next == 'o' ) {
-            __kmp_place_core_offset = num;
-            return;   // threads_per_core remains default
-        } else if ( *next == '\0' ) {
+        } else if (*next == 't' || *next == 'T') {
+            KMP_DEBUG_ASSERT(flagT == 0);
+            __kmp_place_num_threads_per_core = num;
+            flagC = 1; // num_cores could be skipped ?
+            flagT = 1;
+            next++; // can have core-offset specified after num threads
+        } else if (*next == '\0') {
+            KMP_DEBUG_ASSERT(flagC); // 4x2 means 4 cores 2 threads per core
             __kmp_place_num_threads_per_core = num;
+            return;   // two values provided without letter-designator
+        } else {
+            KMP_WARNING(AffThrPlaceInvalid, name, value);
             return;
-        } else {
-            KMP_WARNING( AffThrPlaceInvalid, name, value );
-            return;
-        }
-    } else if ( *next == ',' || *next == 'x' ) {
-        if ( prev_delim == 2 ) {
-            return; // no sense in the only offset value, thus skip the rest
+        }
+        KMP_WARNING(AffThrPlaceInvalid, name, value);
+    SKIP_WS(next);
+    if (*next == '\0')
+        return;   // " Ns,Nc  " - something like this
+    if (CHECK_DELIM(next)) {
+        next++;   // skip delimiter
+        SKIP_WS(next);
+    }
+
+    // Get third value (could be core-offset, num_cores, num_threads)
+    if (*next >= '0' && *next <= '9') {
+        prev = next;
+        SKIP_DIGITS(next);
+        num = __kmp_str_to_int(prev, *next);
+        SKIP_WS(next);
+        if (*next == 'c' || *next == 'C') {
+            KMP_DEBUG_ASSERT(flagC == 0);
+            __kmp_place_num_cores = num;
+            flagC = 1;
+            next++;
+        } else if (*next == 'o' || *next == 'O') {
+            KMP_DEBUG_ASSERT(flagC);
+            __kmp_place_core_offset = num;
+            next++;
+        } else if (*next == 't' || *next == 'T') {
+            KMP_DEBUG_ASSERT(flagT == 0);
+            __kmp_place_num_threads_per_core = num;
+            if (flagC == 0)
+                return; // num_cores could be skipped (e.g. 2s,4o,2t)
+            flagT = 1;
+            next++; // can have core-offset specified later (e.g. 2s,1c,2t,3o)
+        } else {
+            KMP_WARNING(AffThrPlaceInvalid, name, value);
+            return;
         }
-        KMP_DEBUG_ASSERT( prev_delim == 1 );
-        next++;     // no value for threads_per_core provided
     } else {
-        KMP_WARNING( AffThrPlaceInvalid, name, value );
+        KMP_WARNING(AffThrPlaceInvalid, name, value);
         return;
     }
-    SKIP_WS( next );
-    if ( *next == '\0' ) {
-        return;   // " nC,mT  " - something like this
+    KMP_DEBUG_ASSERT(flagC);
+    SKIP_WS(next);
+    if ( *next == '\0' )
+            return;
+    if (CHECK_DELIM(next)) {
+        next++;   // skip delimiter
+        SKIP_WS(next);
+    }
+
+    // Get 4-th value (could be core-offset, num_threads)
+    if (*next >= '0' && *next <= '9') {
+        prev = next;
+        SKIP_DIGITS(next);
+        num = __kmp_str_to_int(prev, *next);
+        SKIP_WS(next);
+        if (*next == 'o' || *next == 'O') {
+            __kmp_place_core_offset = num;
+            next++;
+        } else if (*next == 't' || *next == 'T') {
+            KMP_DEBUG_ASSERT(flagT == 0);
+            __kmp_place_num_threads_per_core = num;
+            flagT = 1;
+            next++; // can have core-offset specified after num threads
+        } else {
+            KMP_WARNING(AffThrPlaceInvalid, name, value);
+            return;
+        }
+    } else {
+        KMP_WARNING(AffThrPlaceInvalid, name, value);
+        return;
     }
-    if ( ( *next == ',' || *next == 'x' ) && !prev_delim ) {
-        prev_delim = 1;
-        next++;   // skip delimiter after threads_per_core value
-        SKIP_WS( next );
+    SKIP_WS(next);
+    if ( *next == '\0' )
+        return;
+    if (CHECK_DELIM(next)) {
+        next++;   // skip delimiter
+        SKIP_WS(next);
     }
 
-    // Get core offset last if any,
-    // don't bother checking syntax after all data obtained
-    if ( *next >= '0' && *next <= '9' ) {
+    // Get 5-th value (could be core-offset, num_threads)
+    if (*next >= '0' && *next <= '9') {
         prev = next;
-        SKIP_DIGITS( next );
-        num = __kmp_str_to_int( prev, *next );
+        SKIP_DIGITS(next);
+        num = __kmp_str_to_int(prev, *next);
+        SKIP_WS(next);
+        if (*next == 'o' || *next == 'O') {
+            KMP_DEBUG_ASSERT(flagT);
             __kmp_place_core_offset = num;
+        } else if (*next == 't' || *next == 'T') {
+            KMP_DEBUG_ASSERT(flagT == 0);
+            __kmp_place_num_threads_per_core = num;
+        } else {
+            KMP_WARNING(AffThrPlaceInvalid, name, value);
         }
+    } else {
+        KMP_WARNING(AffThrPlaceInvalid, name, value);
+    }
+    return;
+#undef CHECK_DELIM
 }
 
 static void
 __kmp_stg_print_place_threads( kmp_str_buf_t * buffer, char const * name, void * data ) {
-    if ( __kmp_place_num_cores + __kmp_place_num_threads_per_core ) {
+    if (__kmp_place_num_sockets + __kmp_place_num_cores + __kmp_place_num_threads_per_core) {
+        int comma = 0;
         kmp_str_buf_t buf;
-        __kmp_str_buf_init( &buf );
-        if( __kmp_env_format ) {
+        __kmp_str_buf_init(&buf);
+        if(__kmp_env_format)
             KMP_STR_BUF_PRINT_NAME_EX(name);
-        } else {
-            __kmp_str_buf_print( buffer, "   %s='", name );
+        else
+            __kmp_str_buf_print(buffer, "   %s='", name);
+        if (__kmp_place_num_sockets) {
+            __kmp_str_buf_print(&buf, "%ds", __kmp_place_num_sockets);
+            if (__kmp_place_socket_offset)
+                __kmp_str_buf_print(&buf, "@%do", __kmp_place_socket_offset);
+            comma = 1;
+        }
+        if (__kmp_place_num_cores) {
+            __kmp_str_buf_print(&buf, "%s%dc", comma?",":"", __kmp_place_num_cores);
+            if (__kmp_place_core_offset)
+                __kmp_str_buf_print(&buf, "@%do", __kmp_place_core_offset);
+            comma = 1;
         }
-        __kmp_str_buf_print( &buf, "%dC", __kmp_place_num_cores );
-        __kmp_str_buf_print( &buf, "x%dT", __kmp_place_num_threads_per_core );
-        if ( __kmp_place_core_offset ) {
-            __kmp_str_buf_print( &buf, ",%dO", __kmp_place_core_offset );
-        }
+        if (__kmp_place_num_threads_per_core)
+            __kmp_str_buf_print(&buf, "%s%dt", comma?",":"", __kmp_place_num_threads_per_core);
         __kmp_str_buf_print(buffer, "%s'\n", buf.str );
         __kmp_str_buf_free(&buf);
 /*
     } else {
         __kmp_str_buf_print( buffer, "   %s: %s \n", name, KMP_I18N_STR( NotDefined ) );
 */
     }
 }