Page MenuHomePhabricator

Added sockets to the syntax of KMP_PLACE_THREADS environment variable.
ClosedPublic

Authored by jlpeyton on Sep 25 2015, 12:08 PM.

Details

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)]"

Diff Detail

Event Timeline

jlpeyton updated this revision to Diff 35750.Sep 25 2015, 12:08 PM
jlpeyton retitled this revision from to Added sockets to the syntax of KMP_PLACE_THREADS environment variable..
jlpeyton updated this object.
jlpeyton added reviewers: AndreyChurbanov, hfinkel.
jlpeyton set the repository for this revision to rL LLVM.
jlpeyton added a subscriber: openmp-commits.

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 accepted this revision.Oct 6 2015, 11:05 AM
AndreyChurbanov edited edge metadata.

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.

This revision is now accepted and ready to land.Oct 6 2015, 11:05 AM
This revision was automatically updated to reflect the committed changes.