[PATCH 0/4] Properly handle GCC target("march=") (PR71652)

[PATCH 0/4] Properly handle GCC target("march=") (PR71652)

[marxin]

Hello.

Following small patch set targets $subject, where we ICE if someone
uses #pragma GCC target ("arch=generic"). My attempt is to not to
create a new target optimization node in case of a wrong value
of march string. Such approach does not generate multiple errors.

Apart from that, I also improved i386 option handling as mentioned
in: [1]

Patch bootstraps and survives regression tests on powerpc64le-unknown-linux-gnu.

Ready for trunk?
Thanks,
Martin

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71652#c4

marxin (4):
  Fix PR target/71652
  Support crc32 as a i386 target optimization node
  Support movbe as a i386 target optimization node
  Remove fused-madd from documentation

 gcc/config/i386/i386.c                    | 64 ++++++++++++++++++++++---------
 gcc/doc/extend.texi                       |  5 ---
 gcc/testsuite/gcc.target/i386/crc32-5.c   | 25 ++++++++++++
 gcc/testsuite/gcc.target/i386/movbe-4.c   | 20 ++++++++++
 gcc/testsuite/gcc.target/i386/pr71652-2.c | 13 +++++++
 gcc/testsuite/gcc.target/i386/pr71652-3.c | 14 +++++++
 gcc/testsuite/gcc.target/i386/pr71652.c   | 13 +++++++
 7 files changed, 130 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/crc32-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/movbe-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652.c

-- 
2.8.4

[PATCH 3/4] Support movbe as a i386 target optimization node

[marxin]

gcc/testsuite/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* gcc.target/i386/movbe-4.c: New test.

gcc/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* config/i386/i386.c (ix86_valid_target_attribute_inner_p):
	Handle movbe.
---
 gcc/config/i386/i386.c                  |  1 +
 gcc/testsuite/gcc.target/i386/movbe-4.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/movbe-4.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 493b7e6..8a30cf2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6437,6 +6437,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
     IX86_ATTR_ISA ("mmx",	OPT_mmmx),
     IX86_ATTR_ISA ("pclmul",	OPT_mpclmul),
     IX86_ATTR_ISA ("popcnt",	OPT_mpopcnt),
+    IX86_ATTR_ISA ("movbe",	OPT_mmovbe),
     IX86_ATTR_ISA ("crc32",	OPT_mcrc32),
     IX86_ATTR_ISA ("sse",	OPT_msse),
     IX86_ATTR_ISA ("sse2",	OPT_msse2),
diff --git a/gcc/testsuite/gcc.target/i386/movbe-4.c b/gcc/testsuite/gcc.target/i386/movbe-4.c
new file mode 100644
index 0000000..9067091
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/movbe-4.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#pragma GCC target ("movbe")
+
+extern int x;
+
+void
+foo (int i)
+{
+  x = __builtin_bswap32 (i);
+}
+
+int
+bar ()
+{
+  return __builtin_bswap32 (x);
+}
+
+/* { dg-final { scan-assembler-times "movbe\[ \t\]" 2 } } */
-- 
2.8.4

[PATCH 2/4] Support crc32 as a i386 target optimization node

[marxin]

gcc/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* config/i386/i386.c (ix86_valid_target_attribute_inner_p):
	Handle crc32.

gcc/testsuite/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* gcc.target/i386/crc32-5.c: New test.
---
 gcc/config/i386/i386.c                  |  1 +
 gcc/testsuite/gcc.target/i386/crc32-5.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/crc32-5.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index c838790..493b7e6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6437,6 +6437,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
     IX86_ATTR_ISA ("mmx",	OPT_mmmx),
     IX86_ATTR_ISA ("pclmul",	OPT_mpclmul),
     IX86_ATTR_ISA ("popcnt",	OPT_mpopcnt),
+    IX86_ATTR_ISA ("crc32",	OPT_mcrc32),
     IX86_ATTR_ISA ("sse",	OPT_msse),
     IX86_ATTR_ISA ("sse2",	OPT_msse2),
     IX86_ATTR_ISA ("sse3",	OPT_msse3),
diff --git a/gcc/testsuite/gcc.target/i386/crc32-5.c b/gcc/testsuite/gcc.target/i386/crc32-5.c
new file mode 100644
index 0000000..a47f1e2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/crc32-5.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler "crc32b\[^\\n\]*eax" } } */
+/* { dg-final { scan-assembler "crc32w\[^\\n\]*eax" } } */
+/* { dg-final { scan-assembler "crc32l\[^\\n\]*eax" } } */
+
+#pragma GCC target ("crc32")
+
+unsigned int
+crc32b (unsigned int x, unsigned char y)
+{
+  return __builtin_ia32_crc32qi (x, y);
+}
+
+unsigned int
+crc32w (unsigned int x, unsigned short y)
+{
+  return __builtin_ia32_crc32hi (x, y);
+}
+
+unsigned int
+crc32d (unsigned int x, unsigned int y)
+{
+  return __builtin_ia32_crc32si (x, y);
+}
-- 
2.8.4

[PATCH 1/4] Fix PR target/71652

[marxin]

gcc/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	PR target/71652
	* config/i386/i386.c (ix86_option_override_internal): Change
	signature and return false when there's an error related to
	arch string.
	(release_options_strings): New function.
	(ix86_valid_target_attribute_tree): Call the function.

gcc/testsuite/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* gcc.target/i386/pr71652.c: New test.
	* gcc.target/i386/pr71652-2.c: New test.
	* gcc.target/i386/pr71652-3.c: New test.
---
 gcc/config/i386/i386.c                    | 62 +++++++++++++++++++++----------
 gcc/testsuite/gcc.target/i386/pr71652-2.c | 13 +++++++
 gcc/testsuite/gcc.target/i386/pr71652-3.c | 14 +++++++
 gcc/testsuite/gcc.target/i386/pr71652.c   | 13 +++++++
 4 files changed, 83 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr71652.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ba35dce..c838790 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4698,9 +4698,10 @@ ix86_override_options_after_change (void)
 
 /* Override various settings based on options.  If MAIN_ARGS_P, the
    options are from the command line, otherwise they are from
-   attributes.  */
+   attributes.  Return true if there's an error related to march
+   option.  */
 
-static void
+static bool
 ix86_option_override_internal (bool main_args_p,
 			       struct gcc_options *opts,
 			       struct gcc_options *opts_set)
@@ -5243,16 +5244,32 @@ ix86_option_override_internal (bool main_args_p,
   for (i = 0; i < pta_size; i++)
     if (! strcmp (opts->x_ix86_arch_string, processor_alias_table[i].name))
       {
+	if (!strcmp (opts->x_ix86_arch_string, "generic"))
+	  {
+	    error ("generic CPU can be used only for %stune=%s %s",
+		   prefix, suffix, sw);
+	    return false;
+	  }
+	else if (!strcmp (opts->x_ix86_arch_string, "intel"))
+	  {
+	    error ("intel CPU can be used only for %stune=%s %s",
+		   prefix, suffix, sw);
+	    return false;
+	  }
+
+	if (TARGET_64BIT_P (opts->x_ix86_isa_flags)
+	    && !(processor_alias_table[i].flags & PTA_64BIT))
+	  {
+	    error ("CPU you selected does not support x86-64 "
+		   "instruction set");
+	    return false;
+	  }
+
 	ix86_schedule = processor_alias_table[i].schedule;
 	ix86_arch = processor_alias_table[i].processor;
 	/* Default cpu tuning to the architecture.  */
 	ix86_tune = ix86_arch;
 
-	if (TARGET_64BIT_P (opts->x_ix86_isa_flags)
-	    && !(processor_alias_table[i].flags & PTA_64BIT))
-	  error ("CPU you selected does not support x86-64 "
-		 "instruction set");
-
 	if (processor_alias_table[i].flags & PTA_MMX
 	    && !(opts->x_ix86_isa_flags_explicit & OPTION_MASK_ISA_MMX))
 	  opts->x_ix86_isa_flags |= OPTION_MASK_ISA_MMX;
@@ -5450,13 +5467,7 @@ ix86_option_override_internal (bool main_args_p,
   if (TARGET_X32 && (ix86_isa_flags & OPTION_MASK_ISA_MPX))
     error ("Intel MPX does not support x32");
 
-  if (!strcmp (opts->x_ix86_arch_string, "generic"))
-    error ("generic CPU can be used only for %stune=%s %s",
-	   prefix, suffix, sw);
-  else if (!strcmp (opts->x_ix86_arch_string, "intel"))
-    error ("intel CPU can be used only for %stune=%s %s",
-	   prefix, suffix, sw);
-  else if (i == pta_size)
+  if (i == pta_size)
     error ("bad value (%s) for %sarch=%s %s",
 	   opts->x_ix86_arch_string, prefix, suffix, sw);
 
@@ -6045,6 +6056,8 @@ ix86_option_override_internal (bool main_args_p,
       ix86_parse_stringop_strategy_string (str, true);
       free (str);
     }
+
+  return true;
 }
 
 /* Implement the TARGET_OPTION_OVERRIDE hook.  */
@@ -6639,6 +6652,15 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
   return ret;
 }
 
+/* Release allocated strings.  */
+static void
+release_options_strings (char **option_strings)
+{
+  /* Free up memory allocated to hold the strings */
+  for (unsigned i = 0; i < IX86_FUNCTION_SPECIFIC_MAX; i++)
+    free (option_strings[i]);
+}
+
 /* Return a TARGET_OPTION_NODE tree of the target options listed or NULL.  */
 
 tree
@@ -6653,7 +6675,6 @@ ix86_valid_target_attribute_tree (tree args,
   int orig_arch_specified = ix86_arch_specified;
   char *option_strings[IX86_FUNCTION_SPECIFIC_MAX] = { NULL, NULL };
   tree t = NULL_TREE;
-  int i;
   struct cl_target_option *def
     = TREE_TARGET_OPTION (target_option_default_node);
   struct gcc_options enum_opts_set;
@@ -6714,7 +6735,12 @@ ix86_valid_target_attribute_tree (tree args,
 	}
 
       /* Do any overrides, such as arch=xxx, or tune=xxx support.  */
-      ix86_option_override_internal (false, opts, opts_set);
+      bool r = ix86_option_override_internal (false, opts, opts_set);
+      if (!r)
+	{
+	  release_options_strings (option_strings);
+	  return error_mark_node;
+	}
 
       /* Add any builtin functions with the new isa if any.  */
       ix86_add_new_builtins (opts->x_ix86_isa_flags);
@@ -6727,9 +6753,7 @@ ix86_valid_target_attribute_tree (tree args,
       opts->x_ix86_tune_string = orig_tune_string;
       opts_set->x_ix86_fpmath = orig_fpmath_set;
 
-      /* Free up memory allocated to hold the strings */
-      for (i = 0; i < IX86_FUNCTION_SPECIFIC_MAX; i++)
-	free (option_strings[i]);
+      release_options_strings (option_strings);
     }
 
   return t;
diff --git a/gcc/testsuite/gcc.target/i386/pr71652-2.c b/gcc/testsuite/gcc.target/i386/pr71652-2.c
new file mode 100644
index 0000000..1dc071d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr71652-2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+
+#pragma GCC push_options
+#pragma GCC target ("arch=intel") /* { dg-error "intel CPU can be used only for option\\(\"tune=\"\\) attribute" } */
+
+__attribute__((constructor)) void foo()
+{
+  asm ("");
+}
+
+#pragma GCC pop_options
+
+int main() { return 0; }
diff --git a/gcc/testsuite/gcc.target/i386/pr71652-3.c b/gcc/testsuite/gcc.target/i386/pr71652-3.c
new file mode 100644
index 0000000..ba99a3e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr71652-3.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-march=haswell" } */
+
+#pragma GCC push_options
+#pragma GCC target ("arch=geode") /* { dg-error "CPU you selected does not support x86-64 instruction set" } */
+
+__attribute__((constructor)) void foo()
+{
+  asm ("");
+}
+
+#pragma GCC pop_options
+
+int main() { return 0; }
diff --git a/gcc/testsuite/gcc.target/i386/pr71652.c b/gcc/testsuite/gcc.target/i386/pr71652.c
new file mode 100644
index 0000000..22503d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr71652.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+
+#pragma GCC push_options
+#pragma GCC target ("arch=generic") /* { dg-error "generic CPU can be used only for option\\(\"tune=\"\\) attribute" } */
+
+__attribute__((constructor)) void foo()
+{
+  asm ("");
+}
+
+#pragma GCC pop_options
+
+int main() { return 0; }
-- 
2.8.4

[PATCH 4/4] Remove fused-madd from documentation

[marxin]

gcc/ChangeLog:

2016-07-18  Martin Liska  <mliska@suse.cz>

	* doc/extend.texi: Remove fused-madd from i386 target
	options.
---
 gcc/doc/extend.texi | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 5b9e617..30957ce 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5443,11 +5443,6 @@ Enable/disable the generation of the CLD before string moves.
 Enable/disable the generation of the @code{sin}, @code{cos}, and
 @code{sqrt} instructions on the 387 floating-point unit.
 
-@item fused-madd
-@itemx no-fused-madd
-@cindex @code{target("fused-madd")} function attribute, x86
-Enable/disable the generation of the fused multiply/add instructions.
-
 @item ieee-fp
 @itemx no-ieee-fp
 @cindex @code{target("ieee-fp")} function attribute, x86
-- 
2.8.4

Re: [PATCH 0/4] Properly handle GCC target("march=") (PR71652)

[Martin Liška]

PING^1

> Hello.
> 
> Following small patch set targets $subject, where we ICE if someone
> uses #pragma GCC target ("arch=generic"). My attempt is to not to
> create a new target optimization node in case of a wrong value
> of march string. Such approach does not generate multiple errors.
> 
> Apart from that, I also improved i386 option handling as mentioned
> in: [1]
> 
> Patch bootstraps and survives regression tests on powerpc64le-unknown-linux-gnu.
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71652#c4
> 
> marxin (4):
>   Fix PR target/71652
>   Support crc32 as a i386 target optimization node
>   Support movbe as a i386 target optimization node
>   Remove fused-madd from documentation
> 
>  gcc/config/i386/i386.c                    | 64 ++++++++++++++++++++++---------
>  gcc/doc/extend.texi                       |  5 ---
>  gcc/testsuite/gcc.target/i386/crc32-5.c   | 25 ++++++++++++
>  gcc/testsuite/gcc.target/i386/movbe-4.c   | 20 ++++++++++
>  gcc/testsuite/gcc.target/i386/pr71652-2.c | 13 +++++++
>  gcc/testsuite/gcc.target/i386/pr71652-3.c | 14 +++++++
>  gcc/testsuite/gcc.target/i386/pr71652.c   | 13 +++++++
>  7 files changed, 130 insertions(+), 24 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/crc32-5.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/movbe-4.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-3.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652.c
> 

Re: [PATCH 0/4] Properly handle GCC target("march=") (PR71652)

[jh]

OK for patches 1-4.

Honza

Dne 2016-08-12 14:15, Martin Liška napsal:
> PING^1
> 
>> Hello.
>> 
>> Following small patch set targets $subject, where we ICE if someone
>> uses #pragma GCC target ("arch=generic"). My attempt is to not to
>> create a new target optimization node in case of a wrong value
>> of march string. Such approach does not generate multiple errors.
>> 
>> Apart from that, I also improved i386 option handling as mentioned
>> in: [1]
>> 
>> Patch bootstraps and survives regression tests on 
>> powerpc64le-unknown-linux-gnu.
>> 
>> Ready for trunk?
>> Thanks,
>> Martin
>> 
>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71652#c4
>> 
>> marxin (4):
>>   Fix PR target/71652
>>   Support crc32 as a i386 target optimization node
>>   Support movbe as a i386 target optimization node
>>   Remove fused-madd from documentation
>> 
>>  gcc/config/i386/i386.c                    | 64 
>> ++++++++++++++++++++++---------
>>  gcc/doc/extend.texi                       |  5 ---
>>  gcc/testsuite/gcc.target/i386/crc32-5.c   | 25 ++++++++++++
>>  gcc/testsuite/gcc.target/i386/movbe-4.c   | 20 ++++++++++
>>  gcc/testsuite/gcc.target/i386/pr71652-2.c | 13 +++++++
>>  gcc/testsuite/gcc.target/i386/pr71652-3.c | 14 +++++++
>>  gcc/testsuite/gcc.target/i386/pr71652.c   | 13 +++++++
>>  7 files changed, 130 insertions(+), 24 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/i386/crc32-5.c
>>  create mode 100644 gcc/testsuite/gcc.target/i386/movbe-4.c
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-2.c
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652-3.c
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr71652.c
>>