On Tue, Jul 21, 2015 at 01:14:44PM +0200, Ulrich Weigand wrote: > Dominik Vogt wrote: > This version is looking good, except for one problem: > > > - "%{!m31:%{!m64:-m64}}", \ > > - "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}}", \ > > - "%{!march=*:%{mesa:-march=g5}%{mzarch:-march=z900}}", \ > > There's a reason for this particular sequence. The first line ensures > that one of -m64 or -m31 is present. The second line ensures that one > of -mesa or -mzarch is present, but this works only if already one of > -m64 or -m31 is present, so it needs to come *after* the first line. > The third line ensures that some -march= switch is present, but this > works only if already one of -mesa or -mzarch is present, so it needs > to comer *after* the second line. > > > +#define DRIVER_SELF_SPECS \ > > + "%{!m31:%{!m64:-m" S390_TARGET_BITS_STRING "}} " \ > > + "%{!march=*:%{mesa:-march=g5}%{mzarch:-march=z900}} " \ > > + MARCH_MTUNE_NATIVE_SPECS \ > > + "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}} " > > This inverts the order of the second and third lines, so it is now > no longer guaranteed that at least one -march= switch is present. > > I understand that you need to move MARCH_MTUNE_NATIVE_SPECS ahead > of the -mesa/-mzarch defaulting rule, but it should be possible > to do that without changing the sequence of the three existing > rules. Why not just move MARCH_MTUNE_NATIVE_SPECS first?
Okay, good point. Since this is a bit complicated, let's consider which effect this change would have on the following two cases in conjunction with -march=native: 1. local_cpu_detect returns -march=<something> Before suggested change: -march=<something> takes effect After change: -march=<something> takes effect => Correct behaviour in both cases, the second line never does anything. 2. local_cpu_detect returns nothing (empty string) Before change: No architecture string is used at all -> Bad! After change: The default defined by the second line kicks in. So the current code has even another bug, apart from the issue you have mentioned. Given the above line numbering (1, 2, 3, 4), the order of activation of the spec rules should be * 1, 4 with -march=<something> (<something> != native) * 1, 4, 2 without -march=<something> * 1, 3, 4 with -march=native (if local_cpu_detect returns a string) * 1, 3, 4, 2 with -march=native (if local_cpu_detect returns nothing) So, the suggested order 1, 3, 4, 2 should cover all these cases. Also, there's another bug. The spec lines need to be separated by commas. This change is also contained in the attached new patch (v4). Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
gcc/ChangeLog * config/s390/driver-native.c (s390_host_detect_local_cpu): Handle processor capabilities with -march=native. * config/s390/s390.h (MARCH_MTUNE_NATIVE_SPECS): Likewise. (DRIVER_SELF_SPECS): Likewise. Join specs for 31 and 64 bit. * (S390_TARGET_BITS_STRING): Macro to simplify specs.
>From 1fc834c4d5d3ac9e26cad101b8a16900a83f4ca0 Mon Sep 17 00:00:00 2001 From: Dominik Vogt <v...@linux.vnet.ibm.com> Date: Mon, 6 Jul 2015 16:28:32 +0100 Subject: [PATCH 3/4] S390: Handle processor capabilities with -march=native. --- gcc/config/s390/driver-native.c | 143 ++++++++++++++++++++++++++++++++-------- gcc/config/s390/s390.h | 28 ++++---- 2 files changed, 129 insertions(+), 42 deletions(-) diff --git a/gcc/config/s390/driver-native.c b/gcc/config/s390/driver-native.c index 88c76bd..5f7fe0a 100644 --- a/gcc/config/s390/driver-native.c +++ b/gcc/config/s390/driver-native.c @@ -42,6 +42,16 @@ s390_host_detect_local_cpu (int argc, const char **argv) char buf[256]; FILE *f; bool arch; + const char *options = ""; + unsigned int has_features; + unsigned int has_processor; + unsigned int is_cpu_z9_109 = 0; + unsigned int has_highgprs = 0; + unsigned int has_dfp = 0; + unsigned int has_te = 0; + unsigned int has_vx = 0; + unsigned int has_opt_esa_zarch = 0; + int i; if (argc < 1) return NULL; @@ -49,43 +59,120 @@ s390_host_detect_local_cpu (int argc, const char **argv) arch = strcmp (argv[0], "arch") == 0; if (!arch && strcmp (argv[0], "tune")) return NULL; + for (i = 1; i < argc; i++) + if (strcmp (argv[i], "mesa_mzarch") == 0) + has_opt_esa_zarch = 1; f = fopen ("/proc/cpuinfo", "r"); if (f == NULL) return NULL; - while (fgets (buf, sizeof (buf), f) != NULL) - if (strncmp (buf, "processor", sizeof ("processor") - 1) == 0) - { - if (strstr (buf, "machine = 9672") != NULL) - cpu = "g5"; - else if (strstr (buf, "machine = 2064") != NULL - || strstr (buf, "machine = 2066") != NULL) - cpu = "z900"; - else if (strstr (buf, "machine = 2084") != NULL - || strstr (buf, "machine = 2086") != NULL) - cpu = "z990"; - else if (strstr (buf, "machine = 2094") != NULL - || strstr (buf, "machine = 2096") != NULL) - cpu = "z9-109"; - else if (strstr (buf, "machine = 2097") != NULL - || strstr (buf, "machine = 2098") != NULL) - cpu = "z10"; - else if (strstr (buf, "machine = 2817") != NULL - || strstr (buf, "machine = 2818") != NULL) - cpu = "z196"; - else if (strstr (buf, "machine = 2827") != NULL - || strstr (buf, "machine = 2828") != NULL) - cpu = "zEC12"; - else if (strstr (buf, "machine = 2964") != NULL) - cpu = "z13"; - break; - } + for (has_features = 0, has_processor = 0; + (has_features == 0 || has_processor == 0) + && fgets (buf, sizeof (buf), f) != NULL; ) + { + if (has_processor == 0 && strncmp (buf, "processor", 9) == 0) + { + const char *p; + long machine_id; + + p = strstr (buf, "machine = "); + if (p == NULL) + continue; + p += 10; + has_processor = 1; + machine_id = strtol (p, NULL, 16); + switch (machine_id) + { + case 0x9672: + cpu = "g5"; + break; + case 0x2064: + case 0x2066: + cpu = "z900"; + break; + case 0x2084: + case 0x2086: + cpu = "z990"; + break; + case 0x2094: + case 0x2096: + cpu = "z9-109"; + is_cpu_z9_109 = 1; + break; + case 0x2097: + case 0x2098: + cpu = "z10"; + break; + case 0x2817: + case 0x2818: + cpu = "z196"; + break; + case 0x2827: + case 0x2828: + cpu = "zEC12"; + break; + case 0x2964: + cpu = "z13"; + break; + } + } + if (has_features == 0 && strncmp (buf, "features", 8) == 0) + { + const char *p; + + p = strchr (buf, ':'); + if (p == NULL) + continue; + p++; + while (*p != 0) + { + int i; + + while (ISSPACE (*p)) + p++; + for (i = 0; !ISSPACE (p[i]) && p[i] != 0; i++) + ; + if (i == 3 && strncmp (p, "dfp", 3) == 0) + has_dfp = 1; + else if (i == 2 && strncmp (p, "te", 2) == 0) + has_te = 1; + else if (i == 2 && strncmp (p, "vx", 2) == 0) + has_vx = 1; + else if (i == 8 && strncmp (p, "highgprs", 8) == 0) + has_highgprs = 1; + p += i; + } + has_features = 1; + } + } fclose (f); if (cpu == NULL) return NULL; - return concat ("-m", argv[0], "=", cpu, NULL); + if (arch) + { + const char *opt_htm = ""; + const char *opt_vx = ""; + const char *opt_esa_zarch = ""; + + /* We may switch off these cpu features but never switch the on + explicitly. This overrides options specified on the command line. */ + if (!has_te) + opt_htm = " -mno-htm"; + if (!has_vx) + opt_vx = " -mno-vx"; + /* However, we set -mzarch only if neither -mzarch nor -mesa are used on + the command line. This allows the user to switch to -mesa manually. + */ + if (!has_opt_esa_zarch && has_highgprs) + opt_esa_zarch = " -mzarch"; + options = concat (options, opt_htm, opt_vx, opt_esa_zarch, NULL); + } + if (has_dfp && is_cpu_z9_109) + cpu = "z9-ec"; + + return concat ("-m", argv[0], "=", cpu, options, NULL); } diff --git a/gcc/config/s390/s390.h b/gcc/config/s390/s390.h index f18b973..66d6702 100644 --- a/gcc/config/s390/s390.h +++ b/gcc/config/s390/s390.h @@ -131,28 +131,28 @@ extern const char *s390_host_detect_local_cpu (int argc, const char **argv); # define EXTRA_SPEC_FUNCTIONS \ { "local_cpu_detect", s390_host_detect_local_cpu }, -# define MARCH_MTUNE_NATIVE_SPECS \ - " %{march=native:%<march=native %:local_cpu_detect(arch)}" \ - " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}" +#define MARCH_MTUNE_NATIVE_SPECS \ + "%{mtune=native:%<mtune=native %:local_cpu_detect(tune)} " \ + "%{march=native:%<march=native" \ + " %:local_cpu_detect(arch %{mesa|mzarch:mesa_mzarch})}" #else # define MARCH_MTUNE_NATIVE_SPECS "" #endif -/* Defaulting rules. */ #ifdef DEFAULT_TARGET_64BIT -#define DRIVER_SELF_SPECS \ - "%{!m31:%{!m64:-m64}}", \ - "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}}", \ - "%{!march=*:%{mesa:-march=g5}%{mzarch:-march=z900}}", \ - MARCH_MTUNE_NATIVE_SPECS +#define S390_TARGET_BITS_STRING "64" #else -#define DRIVER_SELF_SPECS \ - "%{!m31:%{!m64:-m31}}", \ - "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}}", \ - "%{!march=*:%{mesa:-march=g5}%{mzarch:-march=z900}}", \ - MARCH_MTUNE_NATIVE_SPECS +#define S390_TARGET_BITS_STRING "31" #endif +/* Defaulting rules. */ +#define DRIVER_SELF_SPECS \ + "%{!m31:%{!m64:-m" S390_TARGET_BITS_STRING "}} ", \ + MARCH_MTUNE_NATIVE_SPECS, \ + "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}} ", \ + "%{!march=*:%{mesa:-march=g5}%{mzarch:-march=z900}} " + + /* Constants needed to control the TEST DATA CLASS (TDC) instruction. */ #define S390_TDC_POSITIVE_ZERO (1 << 11) #define S390_TDC_NEGATIVE_ZERO (1 << 10) -- 2.3.0