public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC,PATCH] Provide get_attr functions with alternative parameter
@ 2007-08-06 15:30 Andreas Krebbel
  2007-08-06 15:47 ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Krebbel @ 2007-08-06 15:30 UTC (permalink / raw)
  To: gcc-patches

Hello,

I'm working on a patch which has to draw an insn attribute of a
certain alternative into account. Unfortunately the existing getter
functions for insn attributes only return an attribute value for the
matching alternative.

The attached patch provides getter functions with an additional
argument specifying the number of the alternative for which the
attribute value should be returned. In order to get the old behaviour
the functions need to be invoked with -1 as alternative number. Inline
wrappers with the old function names are provided in order to keep
backwards compatibility.

Does this look like the right approach?

Bye,

-Andreas-

Index: gcc/genattr.c
===================================================================
*** gcc/genattr.c.orig	2007-01-16 13:47:34.000000000 +0100
--- gcc/genattr.c	2007-08-06 16:04:26.000000000 +0200
*************** gen_attr (rtx attr)
*** 51,58 ****
    /* If numeric attribute, don't need to write an enum.  */
    p = XSTR (attr, 1);
    if (*p == '\0')
!     printf ("extern int get_attr_%s (%s);\n", XSTR (attr, 0),
! 	    (is_const ? "void" : "rtx"));
    else
      {
        printf ("enum attr_%s {", XSTR (attr, 0));
--- 51,73 ----
    /* If numeric attribute, don't need to write an enum.  */
    p = XSTR (attr, 1);
    if (*p == '\0')
!     {
!       if (!strcmp (XSTR (attr, 0), "length"))
! 	printf ("extern int get_attr_length (rtx);\n");
!       else
! 	{
! 	  printf ("extern int get_attr_alt_%s (%s);\n", XSTR (attr, 0),
! 		  (is_const ? "void" : "rtx, int"));
! 	  if (!is_const)
! 	    {
! 	      printf ("static inline int get_attr_%s (rtx);\n\n\n", XSTR (attr, 0));
! 	      printf ("static inline int get_attr_%s (rtx insn)\n",
! 		      XSTR (attr, 0));
! 	      printf ("{\n  return get_attr_alt_%s (insn, -1);\n}\n\n",
! 		      XSTR (attr, 0));
! 	    }
! 	}
!     }
    else
      {
        printf ("enum attr_%s {", XSTR (attr, 0));
*************** gen_attr (rtx attr)
*** 68,75 ****
  	}
  
        fputs ("};\n", stdout);
!       printf ("extern enum attr_%s get_attr_%s (%s);\n\n",
! 	      XSTR (attr, 0), XSTR (attr, 0), (is_const ? "void" : "rtx"));
      }
  
    /* If `length' attribute, write additional function definitions and define
--- 83,99 ----
  	}
  
        fputs ("};\n", stdout);
!       printf ("extern enum attr_%s get_attr_alt_%s (%s);\n\n\n",
! 	      XSTR (attr, 0), XSTR (attr, 0), (is_const ? "void" : "rtx, int"));
!       if (!is_const)
! 	{
! 	  printf ("static inline enum attr_%s get_attr_%s (rtx);\n",
! 		  XSTR (attr, 0), XSTR (attr, 0));
! 	  printf ("static inline enum attr_%s get_attr_%s (rtx insn)\n",
! 		  XSTR (attr, 0), XSTR (attr, 0));
! 	  printf ("{\n  return get_attr_alt_%s (insn, -1);\n}\n\n",
! 		  XSTR (attr, 0));
! 	}
      }
  
    /* If `length' attribute, write additional function definitions and define
*************** gen_attr (rtx attr)
*** 78,87 ****
      {
        puts ("\
  extern void shorten_branches (rtx);\n\
! extern int insn_default_length (rtx);\n\
! extern int insn_min_length (rtx);\n\
! extern int insn_variable_length_p (rtx);\n\
! extern int insn_current_length (rtx);\n\n\
  #include \"insn-addr.h\"\n");
      }
  }
--- 102,135 ----
      {
        puts ("\
  extern void shorten_branches (rtx);\n\
! extern int insn_alt_default_length (rtx, int);\n\
! extern int insn_alt_min_length (rtx, int);\n\
! extern int insn_alt_variable_length_p (rtx, int);\n\
! extern int insn_alt_current_length (rtx, int);\n\n\
! static inline int insn_default_length (rtx);\n\
! static inline int insn_min_length (rtx);\n\
! static inline int insn_variable_length_p (rtx);\n\
! static inline int insn_current_length (rtx);\n\n\
! static inline int\n\
! insn_default_length (rtx insn)\n\
! {\n\
!   return insn_alt_default_length (insn, -1);\n\
! }\n\
! static inline int\n\
! insn_min_length (rtx insn)\n\
! {\n\
!   return insn_alt_min_length (insn, -1);\n\
! }\n\
! static inline int\n\
! insn_variable_length_p (rtx insn)\n\
! {\n\
!   return insn_alt_variable_length_p (insn, -1);\n\
! }\n\
! static inline int\n\
! insn_current_length (rtx insn)\n\
! {\n\
!   return insn_alt_current_length (insn, -1);\n\
! }\n\n\
  #include \"insn-addr.h\"\n");
      }
  }
*************** main (int argc, char **argv)
*** 175,186 ****
        printf ("#endif\n\n");
        /* Interface itself: */
        printf ("/* Internal insn code number used by automata.  */\n");
!       printf ("extern int internal_dfa_insn_code (rtx);\n\n");
        printf ("/* Insn latency time defined in define_insn_reservation. */\n");
!       printf ("extern int insn_default_latency (rtx);\n\n");
        printf ("/* Return nonzero if there is a bypass for given insn\n");
        printf ("   which is a data producer.  */\n");
!       printf ("extern int bypass_p (rtx);\n\n");
        printf ("/* Insn latency time on data consumed by the 2nd insn.\n");
        printf ("   Use the function if bypass_p returns nonzero for\n");
        printf ("   the 1st insn. */\n");
--- 223,252 ----
        printf ("#endif\n\n");
        /* Interface itself: */
        printf ("/* Internal insn code number used by automata.  */\n");
!       printf ("static inline int internal_dfa_insn_code (rtx);\n\n");
!       printf ("extern int internal_alt_dfa_insn_code (rtx, int);\n\n");
!       printf ("static inline int\n");
!       printf ("internal_dfa_insn_code (rtx insn)\n");
!       printf ("{\n");
!       printf ("  return internal_alt_dfa_insn_code (insn, -1);\n");
!       printf ("}\n\n");
        printf ("/* Insn latency time defined in define_insn_reservation. */\n");
!       printf ("static inline int insn_default_latency (rtx);\n\n");
!       printf ("extern int insn_alt_default_latency (rtx, int);\n\n");
!       printf ("static inline int\n");
!       printf ("insn_default_latency (rtx insn)\n");
!       printf ("{\n");
!       printf ("  return insn_alt_default_latency (insn, -1);\n");
!       printf ("}\n\n");
        printf ("/* Return nonzero if there is a bypass for given insn\n");
        printf ("   which is a data producer.  */\n");
!       printf ("static inline int bypass_p (rtx);\n\n");
!       printf ("extern int bypass_alt_p (rtx, int);\n\n");
!       printf ("static inline int\n");
!       printf ("bypass_p (rtx insn)\n");
!       printf ("{\n");
!       printf ("  return bypass_alt_p (insn, -1);\n");
!       printf ("}\n\n");
        printf ("/* Insn latency time on data consumed by the 2nd insn.\n");
        printf ("   Use the function if bypass_p returns nonzero for\n");
        printf ("   the 1st insn. */\n");
Index: gcc/genattrtab.c
===================================================================
*** gcc/genattrtab.c.orig	2007-02-19 08:41:15.000000000 +0100
--- gcc/genattrtab.c	2007-08-06 16:03:58.000000000 +0200
*************** make_length_attrs (void)
*** 1531,1540 ****
  {
    static const char *new_names[] =
      {
!       "*insn_default_length",
!       "*insn_min_length",
!       "*insn_variable_length_p",
!       "*insn_current_length"
      };
    static rtx (*const no_address_fn[]) (rtx)
      = {identity_fn,identity_fn, zero_fn, zero_fn};
--- 1531,1540 ----
  {
    static const char *new_names[] =
      {
!       "*insn_alt_default_length",
!       "*insn_alt_min_length",
!       "*insn_alt_variable_length_p",
!       "*insn_alt_current_length"
      };
    static rtx (*const no_address_fn[]) (rtx)
      = {identity_fn,identity_fn, zero_fn, zero_fn};
*************** write_test_expr (rtx exp, int flags)
*** 3253,3259 ****
        if (! (flags & 1) && GET_CODE (XEXP (exp, 0)) == EQ_ATTR
  	  && XSTR (XEXP (exp, 0), 0) == alternative_name)
  	{
! 	  printf ("which_alternative != %s", XSTR (XEXP (exp, 0), 1));
  	  break;
  	}
  
--- 3253,3259 ----
        if (! (flags & 1) && GET_CODE (XEXP (exp, 0)) == EQ_ATTR
  	  && XSTR (XEXP (exp, 0), 0) == alternative_name)
  	{
! 	  printf ("alternative != %s", XSTR (XEXP (exp, 0), 1));
  	  break;
  	}
  
*************** write_test_expr (rtx exp, int flags)
*** 3317,3328 ****
  	      if (!(set & 1))
  		bit++;
  
! 	      printf ("which_alternative %s= %d",
  		      XINT (exp, 1) ? "!" : "=", bit);
  	    }
  	  else
  	    {
! 	      printf ("%s((1 << which_alternative) & 0x%x)",
  		      XINT (exp, 1) ? "!" : "", set);
  	    }
  	}
--- 3317,3328 ----
  	      if (!(set & 1))
  		bit++;
  
! 	      printf ("alternative %s= %d",
  		      XINT (exp, 1) ? "!" : "=", bit);
  	    }
  	  else
  	    {
! 	      printf ("%s((1 << alternative) & 0x%x)",
  		      XINT (exp, 1) ? "!" : "", set);
  	    }
  	}
*************** write_test_expr (rtx exp, int flags)
*** 3337,3343 ****
  
        if (XSTR (exp, 0) == alternative_name)
  	{
! 	  printf ("which_alternative == %s", XSTR (exp, 1));
  	  break;
  	}
  
--- 3337,3343 ----
  
        if (XSTR (exp, 0) == alternative_name)
  	{
! 	  printf ("alternative == %s", XSTR (exp, 1));
  	  break;
  	}
  
*************** write_attr_get (struct attr_desc *attr)
*** 3648,3656 ****
    /* If the attribute name starts with a star, the remainder is the name of
       the subroutine to use, instead of `get_attr_...'.  */
    if (attr->name[0] == '*')
!     printf ("%s (rtx insn ATTRIBUTE_UNUSED)\n", &attr->name[1]);
    else if (attr->is_const == 0)
!     printf ("get_attr_%s (rtx insn ATTRIBUTE_UNUSED)\n", attr->name);
    else
      {
        printf ("get_attr_%s (void)\n", attr->name);
--- 3648,3660 ----
    /* If the attribute name starts with a star, the remainder is the name of
       the subroutine to use, instead of `get_attr_...'.  */
    if (attr->name[0] == '*')
!     printf ("%s (rtx insn ATTRIBUTE_UNUSED, "
!             "int alternative ATTRIBUTE_UNUSED)\n",
! 	    &attr->name[1]);
    else if (attr->is_const == 0)
!     printf ("get_attr_alt_%s (rtx insn ATTRIBUTE_UNUSED, "
!             "int alternative ATTRIBUTE_UNUSED)\n",
! 	    attr->name);
    else
      {
        printf ("get_attr_%s (void)\n", attr->name);
*************** write_attr_case (struct attr_desc *attr,
*** 3857,3868 ****
--- 3861,3888 ----
    if (must_constrain)
      {
        write_indent (indent + 2);
+       printf ("if (alternative == -1)\n");
+       write_indent (indent + 2);
+       printf ("{\n");
+       write_indent (indent + 4);
        printf ("extract_constrain_insn_cached (insn);\n");
+       write_indent (indent + 4);
+       printf ("alternative = which_alternative;\n");
+       write_indent (indent + 2);
+       printf ("}\n");
      }
    else if (must_extract)
      {
        write_indent (indent + 2);
+       printf ("if (alternative == -1)\n");
+       write_indent (indent + 2);
+       printf ("{\n");
+       write_indent (indent + 4);
        printf ("extract_insn_cached (insn);\n");
+       write_indent (indent + 4);
+       printf ("alternative = which_alternative;\n");
+       write_indent (indent + 2);
+       printf ("}\n");
      }
  
    if (av->num_insns == 1)
*************** make_automaton_attrs (void)
*** 4429,4437 ****
  	  }
      }
  
!   make_internal_attr ("*internal_dfa_insn_code", code_exp, ATTR_NONE);
!   make_internal_attr ("*insn_default_latency",   lats_exp, ATTR_NONE);
!   make_internal_attr ("*bypass_p",               byps_exp, ATTR_NONE);
  }
  
  int
--- 4449,4457 ----
  	  }
      }
  
!   make_internal_attr ("*internal_alt_dfa_insn_code", code_exp, ATTR_NONE);
!   make_internal_attr ("*insn_alt_default_latency",   lats_exp, ATTR_NONE);
!   make_internal_attr ("*bypass_alt_p",               byps_exp, ATTR_NONE);
  }
  
  int

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC,PATCH] Provide get_attr functions with alternative parameter
  2007-08-06 15:30 [RFC,PATCH] Provide get_attr functions with alternative parameter Andreas Krebbel
@ 2007-08-06 15:47 ` Paolo Bonzini
  2007-08-07 11:45   ` Andreas Krebbel
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2007-08-06 15:47 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches


> The attached patch provides getter functions with an additional
> argument specifying the number of the alternative for which the
> attribute value should be returned. In order to get the old behaviour
> the functions need to be invoked with -1 as alternative number. Inline
> wrappers with the old function names are provided in order to keep
> backwards compatibility.

Interesting!  Is there a reason (in the patch you're working on) why the 
inline wrappers couldn't just do

   extract_constrain_insn_cached (insn);
   return get_alt_attr_blah_blah (insn, which_alternative);

instead of supporting the special alternative number -1 in 
get_alt_attr_blah_blah?  (I can see the reason could be code bloat, or 
GCC deciding not to inline get_attr_blah_blah, but I thought I'd just ask).

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC,PATCH] Provide get_attr functions with alternative parameter
  2007-08-06 15:47 ` Paolo Bonzini
@ 2007-08-07 11:45   ` Andreas Krebbel
  0 siblings, 0 replies; 3+ messages in thread
From: Andreas Krebbel @ 2007-08-07 11:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: gcc-patches

Hello,

> Interesting!  Is there a reason (in the patch you're working on) why the 
> inline wrappers couldn't just do
> 
>   extract_constrain_insn_cached (insn);
>   return get_alt_attr_blah_blah (insn, which_alternative);
> 
> instead of supporting the special alternative number -1 in 
> get_alt_attr_blah_blah?  (I can see the reason could be code bloat, or 
> GCC deciding not to inline get_attr_blah_blah, but I thought I'd just ask).

Good question. The current code emitted by genattrtab only invokes
extract_constrain_insn if the value of the alternative really depends
on the number of the alternative. So for several attributes it might
not get invoked at all. I was just trying to leave that behaviour as
is in order to avoid performance regressions. It is not required by
the patch I'm working on.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-08-07 11:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-06 15:30 [RFC,PATCH] Provide get_attr functions with alternative parameter Andreas Krebbel
2007-08-06 15:47 ` Paolo Bonzini
2007-08-07 11:45   ` Andreas Krebbel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).