This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AArch64][8/14] Implement TARGET_OPTION_VALID_ATTRIBUTE_P



On 04/08/15 09:53, James Greenhalgh wrote:
On Mon, Aug 03, 2015 at 04:20:13PM +0100, Kyrill Tkachov wrote:
Ok, I've removed usages of 'ret' in favor of returning when appropriate.
In this last one I left the ret (but cleaned up the control flow a bit)
because if the processing fails we need to clean up a bit of state before
returning.
This is OK with the changes below fixed, or commented on as justification.

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index fc1cec7..3a5482d 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -376,6 +378,8 @@ extern bool aarch64_madd_needs_nop (rtx_insn *);
  extern void aarch64_final_prescan_insn (rtx_insn *);
  extern bool
  aarch64_expand_vec_perm_const (rtx target, rtx op0, rtx op1, rtx sel);
+bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
+			     const struct cl_decoded_option *, location_t);
Please try to keep this file in alphabetical order, first by return type,
then by function name.

Ok, will do.


  void aarch64_atomic_assign_expand_fenv (tree *, tree *, tree *);
  int aarch64_ccmp_mode_to_code (enum machine_mode mode);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d0d62e7..7a369fd 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
+static bool
+aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
+{
+  const struct processor *tmp_arch = NULL;
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    {
+      gcc_assert (tmp_arch);
+      selected_arch = tmp_arch;
+      explicit_arch = selected_arch->arch;
+      return true;
+    }
Why not pull this in to the switch case below?

I chose to keep the success case separate from error handling and reporting as it made it
easier to find it (and it is the more interesting case in these functions). I can add a comment
to that effect there if you'd like.

Thanks,
Kyrill


+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing architecture name in 'arch' target %s", pragma_or_attr);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier %qs for 'arch' target %s",
+	       str, pragma_or_attr);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+
+  return false;
+}
+
+/* Handle the argument CPU_STR to the cpu= target attribute.
+   PRAGMA_OR_ATTR is used in potential error messages.  */
+
+static bool
+aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
+{
+  const struct processor *tmp_cpu = NULL;
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    {
+      gcc_assert (tmp_cpu);
+      selected_tune = tmp_cpu;
+      explicit_tune_core = selected_tune->ident;
+
+      selected_arch = &all_architectures[tmp_cpu->arch];
+      explicit_arch = selected_arch->arch;
+      return true;
+    }
Likewise here.

+
+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing cpu name in 'cpu' target %s", pragma_or_attr);
+	break;
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
+	break;
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier %qs for 'cpu' target %s",
+	       str, pragma_or_attr);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+
+  return false;
+}
+
+/* Handle the argument STR to the tune= target attribute.
+   PRAGMA_OR_ATTR is used in potential error messages.  */
+
+static bool
+aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
+{
+  const struct processor *tmp_tune = NULL;
+  enum aarch64_parse_opt_result parse_res
+    = aarch64_parse_tune (str, &tmp_tune);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    {
+      gcc_assert (tmp_tune);
+      selected_tune = tmp_tune;
+      explicit_tune_core = selected_tune->ident;
+      return true;
+    }
+
And likewise here.

+  switch (parse_res)
+    {
+      case AARCH64_PARSE_INVALID_ARG:
+	error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
+	break;
+      default:
+	gcc_unreachable ();
+    }
+
+  return false;
+}
+
+/* Parse an architecture extensions target attribute string specified in STR.
+   For example "+fp+nosimd".  Show any errors if needed.  Return TRUE
+   if successful.  Update aarch64_isa_flags to reflect the ISA features
+   modified.
+   PRAGMA_OR_ATTR is used in potential error messages.  */
+
+static bool
+aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
+{
+  enum aarch64_parse_opt_result parse_res;
+  unsigned long isa_flags = aarch64_isa_flags;
+
+  parse_res = aarch64_parse_extension (str, &isa_flags);
+
+  if (parse_res == AARCH64_PARSE_OK)
+    {
+      aarch64_isa_flags = isa_flags;
+      return true;
+    }
+
And again here.

+  switch (parse_res)
+    {
+      case AARCH64_PARSE_MISSING_ARG:
+	error ("missing feature modifier in target %s %qs",
+	       pragma_or_attr, str);
+	break;
+
+      case AARCH64_PARSE_INVALID_FEATURE:
+	error ("invalid feature modifier in target %s %qs",
+	       pragma_or_attr, str);
+	break;
+
+      default:
+	gcc_unreachable ();
+    }
+
+ return false;
+}
+
Thanks,
James



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]