Patchwork RFA: Process '*' in '@'-output-template alternatives

login
register
mail settings
Submitter Joern Rennecke
Date Sept. 19, 2012, 1:51 a.m.
Message ID <20120918215123.xd8ig0fdus4g0c4c-nzlynne@webmail.spamcop.net>
Download mbox | patch
Permalink /patch/184903/
State New
Headers show

Comments

Joern Rennecke - Sept. 19, 2012, 1:51 a.m.
I am about to submit the ARCompact target port; this port needs a few
patches to target-independent code.

There is a move pattern with 20 alternatives; a few of them need a simple
function call to decide which output pattern to use.  With the '@'-syntax
for multi-alternative templates, each alternative is still a one-liner.
Requiring to transform this into some switch statement would make the thing
several times as big, and very hard to take in; besides, it is generally
a maintenance issue if you have to completely rewrite a multi-alternative
template if you just change one alternative from a constant to some C-code,
or vice versa for the last non-literal alternative.

The attached patch makes the '*' syntax for C code fragments available for
individual alternatives of an '@' multi-alternative output template.
It does this by translating the input into a switch statement in the
generated file, so in a way this is just syntactic sugar, but it's syntactic
sugar that makes some machine descriptions easier to write and change.

Bootstrapped in revision 191429 for i686-pc-linux-gnu.

I've been wondering if it'd make sense to also support for '{' / '}' ,
but at least in the ARCompact context, I think the use of that syntax
inside a multi-alternative template would reduce rather than improve
legibility, so, having no application for the '{' / '}' in that place,
there seems to be no use in adding support for that at this point in time.
2008-11-19  J"orn Rennecke  <joern.rennecke@arc.com>

	* genoutput.c (process_template): Process '*' in '@' alternatives.
Richard Guenther - Sept. 19, 2012, 9:11 a.m.
On Wed, Sep 19, 2012 at 3:51 AM, Joern Rennecke <amylaar@spamcop.net> wrote:
> I am about to submit the ARCompact target port; this port needs a few
> patches to target-independent code.
>
> There is a move pattern with 20 alternatives; a few of them need a simple
> function call to decide which output pattern to use.  With the '@'-syntax
> for multi-alternative templates, each alternative is still a one-liner.
> Requiring to transform this into some switch statement would make the thing
> several times as big, and very hard to take in; besides, it is generally
> a maintenance issue if you have to completely rewrite a multi-alternative
> template if you just change one alternative from a constant to some C-code,
> or vice versa for the last non-literal alternative.
>
> The attached patch makes the '*' syntax for C code fragments available for
> individual alternatives of an '@' multi-alternative output template.
> It does this by translating the input into a switch statement in the
> generated file, so in a way this is just syntactic sugar, but it's syntactic
> sugar that makes some machine descriptions easier to write and change.
>
> Bootstrapped in revision 191429 for i686-pc-linux-gnu.
>
> I've been wondering if it'd make sense to also support for '{' / '}' ,
> but at least in the ARCompact context, I think the use of that syntax
> inside a multi-alternative template would reduce rather than improve
> legibility, so, having no application for the '{' / '}' in that place,
> there seems to be no use in adding support for that at this point in time.

I think that needs to be documented somewhere in the internals manual,
possibly with an example.

Richard.

> 2008-11-19  J"orn Rennecke  <joern.rennecke@arc.com>
>
>         * genoutput.c (process_template): Process '*' in '@' alternatives.
>
> Index: genoutput.c
> ===================================================================
> --- genoutput.c (revision 191429)
> +++ genoutput.c (working copy)
> @@ -662,19 +662,55 @@ process_template (struct data *d, const
>       list of assembler code templates, one for each alternative.  */
>    else if (template_code[0] == '@')
>      {
> -      d->template_code = 0;
> -      d->output_format = INSN_OUTPUT_FORMAT_MULTI;
> +      int found_star = 0;
>
> -      printf ("\nstatic const char * const output_%d[] = {\n",
> d->code_number);
> +      for (cp = &template_code[1]; *cp; )
> +       {
> +         while (ISSPACE (*cp))
> +           cp++;
> +         if (*cp == '*')
> +           found_star = 1;
> +         while (!IS_VSPACE (*cp) && *cp != '\0')
> +           ++cp;
> +       }
> +      d->template_code = 0;
> +      if (found_star)
> +       {
> +         d->output_format = INSN_OUTPUT_FORMAT_FUNCTION;
> +         puts ("\nstatic const char *");
> +         printf ("output_%d (rtx *operands ATTRIBUTE_UNUSED, "
> +                 "rtx insn ATTRIBUTE_UNUSED)\n", d->code_number);
> +         puts ("{");
> +         puts ("  switch (which_alternative)\n    {");
> +       }
> +      else
> +       {
> +         d->output_format = INSN_OUTPUT_FORMAT_MULTI;
> +         printf ("\nstatic const char * const output_%d[] = {\n",
> +                 d->code_number);
> +       }
>
>        for (i = 0, cp = &template_code[1]; *cp; )
>         {
> -         const char *ep, *sp;
> +         const char *ep, *sp, *bp;
>
>           while (ISSPACE (*cp))
>             cp++;
>
> -         printf ("  \"");
> +         bp = cp;
> +         if (found_star)
> +           {
> +             printf ("    case %d:", i);
> +             if (*cp == '*')
> +               {
> +                 printf ("\n      ");
> +                 cp++;
> +               }
> +             else
> +               printf (" return \"");
> +           }
> +         else
> +           printf ("  \"");
>
>           for (ep = sp = cp; !IS_VSPACE (*ep) && *ep != '\0'; ++ep)
>             if (!ISSPACE (*ep))
> @@ -690,7 +726,18 @@ process_template (struct data *d, const
>               cp++;
>             }
>
> -         printf ("\",\n");
> +         if (!found_star)
> +           puts ("\",");
> +         else if (*bp != '*')
> +           puts ("\";");
> +         else
> +           {
> +             /* The usual action will end with a return.
> +                If there is neither break or return at the end, this is
> +                assumed to be intentional; this allows to have multiple
> +                consecutive alternatives share some code.  */
> +             puts ("");
> +           }
>           i++;
>         }
>        if (i == 1)
> @@ -700,7 +747,10 @@ process_template (struct data *d, const
>         error_with_line (d->lineno,
>                          "wrong number of alternatives in the output
> template");
>
> -      printf ("};\n");
> +      if (found_star)
> +       puts ("      default: gcc_unreachable ();\n    }\n}");
> +      else
> +       printf ("};\n");
>      }
>    else
>      {
>
Joern Rennecke - Sept. 19, 2012, 12:06 p.m.
Quoting Richard Guenther <richard.guenther@gmail.com>:


> I think that needs to be documented somewhere in the internals manual,

I suppose it should logically go to the current end of the Output  
Statement node in md.texi .  Line 668 in revision 191429, just before  
the Predicates
node.

> possibly with an example.

AFAICT the existing examples are pieces of real machine descriptions.
One possibility would be the movsi_insn from the arc-4_4-20090909-branch:

(define_insn "*movsi_insn"
   [(set (match_operand:SI 0 "move_dest_operand" "=Rcq,Rcq#q,w, w,w,   
w,???w, ?w,  w,Rcq#q, w,Rcq,  S,Us<,RcqRck,!*x,r,m,???m,VUsc")
         (match_operand:SI 1 "move_src_operand"  "  
cL,cP,Rcq#q,cL,I,Crr,?Rac,Cpc,Clb,?Cal,?Cal,T,Rcq,RcqRck,Us>,Usd,m,c,?Rac,C32"))]
   "register_operand (operands[0], SImode)
    || register_operand (operands[1], SImode)
    || (CONSTANT_P (operands[1])
        /* Don't use a LIMM that we could load with a single insn - we loose
           delay-slot filling opportunities.  */
        && !satisfies_constraint_I (operands[1])
        && satisfies_constraint_Usc (operands[0]))"
   "@
    mov%? %0,%1%&
    mov%? %0,%1%&
    mov%? %0,%1%&
    mov%? %0,%1
    mov%? %0,%1
    ror %0,((%1*2+1) & 0x3f)
    mov%? %0,%1
    add %0,%S1
    * return arc_get_unalign () ? \"add %0,pcl,%1-.+2\" : \"add %0,pcl,%1-.\";
    mov%? %0,%S1%&
    mov%? %0,%S1
    ld%? %0,%1%&
    st%? %1,%0%&
    * return arc_short_long (insn, \"push%? %1%&\", \"st%U0 %1,%0%&\");
    * return arc_short_long (insn, \"pop%? %0%&\",  \"ld%U1 %0,%1%&\");
    ld%? %0,%1%&
    ld%U1%V1 %0,%1
    st%U0%V0 %1,%0
    st%U0%V0 %1,%0
    st%U0%V0 %S1,%0"
   [(set_attr "type"  
"move,move,move,move,move,two_cycle_core,move,binary,binary,move,move,load,store,store,load,load,load,store,store,store")
    (set_attr "iscompact"  
"maybe,maybe,maybe,false,false,false,false,false,false,maybe_limm,false,true,true,true,true,true,false,false,false,false")
    ; Use default length for iscompact to mark length varying.  But set length
    ; of Crr to 4.
    (set_attr "length" "*,*,*,4,4,4,4,8,8,*,8,*,*,*,*,*,*,*,*,8")
    (set_attr "cond"  
"canuse,canuse_limm,canuse,canuse,canuse_limm,canuse_limm,canuse,nocond,nocond,canuse,canuse,nocond,nocond,nocond,nocond,nocond,nocond,nocond,nocond,nocond")])

Although the number of different concepts combined here might a bit
distract from the point.  Also, it'll need steering commitee approval
to put this code, which was previously contributed under the GPL (on the
branch) into GFDL documentation.

Or should I make up a reduced/synthetic example for a simpler - but
probably pointless as an actual output template - example?
Richard Guenther - Sept. 19, 2012, 1:16 p.m.
On Wed, Sep 19, 2012 at 2:06 PM, Joern Rennecke <amylaar@spamcop.net> wrote:
> Quoting Richard Guenther <richard.guenther@gmail.com>:
>
>
>> I think that needs to be documented somewhere in the internals manual,
>
>
> I suppose it should logically go to the current end of the Output Statement
> node in md.texi .  Line 668 in revision 191429, just before the Predicates
> node.

Yes.

>> possibly with an example.
>
>
> AFAICT the existing examples are pieces of real machine descriptions.
> One possibility would be the movsi_insn from the arc-4_4-20090909-branch:
>
> (define_insn "*movsi_insn"
>   [(set (match_operand:SI 0 "move_dest_operand" "=Rcq,Rcq#q,w, w,w,  w,???w,
> ?w,  w,Rcq#q, w,Rcq,  S,Us<,RcqRck,!*x,r,m,???m,VUsc")
>         (match_operand:SI 1 "move_src_operand"  "
> cL,cP,Rcq#q,cL,I,Crr,?Rac,Cpc,Clb,?Cal,?Cal,T,Rcq,RcqRck,Us>,Usd,m,c,?Rac,C32"))]
>   "register_operand (operands[0], SImode)
>    || register_operand (operands[1], SImode)
>    || (CONSTANT_P (operands[1])
>        /* Don't use a LIMM that we could load with a single insn - we loose
>           delay-slot filling opportunities.  */
>        && !satisfies_constraint_I (operands[1])
>        && satisfies_constraint_Usc (operands[0]))"
>   "@
>    mov%? %0,%1%&
>    mov%? %0,%1%&
>    mov%? %0,%1%&
>    mov%? %0,%1
>    mov%? %0,%1
>    ror %0,((%1*2+1) & 0x3f)
>    mov%? %0,%1
>    add %0,%S1
>    * return arc_get_unalign () ? \"add %0,pcl,%1-.+2\" : \"add
> %0,pcl,%1-.\";
>    mov%? %0,%S1%&
>    mov%? %0,%S1
>    ld%? %0,%1%&
>    st%? %1,%0%&
>    * return arc_short_long (insn, \"push%? %1%&\", \"st%U0 %1,%0%&\");
>    * return arc_short_long (insn, \"pop%? %0%&\",  \"ld%U1 %0,%1%&\");
>    ld%? %0,%1%&
>    ld%U1%V1 %0,%1
>    st%U0%V0 %1,%0
>    st%U0%V0 %1,%0
>    st%U0%V0 %S1,%0"
>   [(set_attr "type"
> "move,move,move,move,move,two_cycle_core,move,binary,binary,move,move,load,store,store,load,load,load,store,store,store")
>    (set_attr "iscompact"
> "maybe,maybe,maybe,false,false,false,false,false,false,maybe_limm,false,true,true,true,true,true,false,false,false,false")
>    ; Use default length for iscompact to mark length varying.  But set
> length
>    ; of Crr to 4.
>    (set_attr "length" "*,*,*,4,4,4,4,8,8,*,8,*,*,*,*,*,*,*,*,8")
>    (set_attr "cond"
> "canuse,canuse_limm,canuse,canuse,canuse_limm,canuse_limm,canuse,nocond,nocond,canuse,canuse,nocond,nocond,nocond,nocond,nocond,nocond,nocond,nocond,nocond")])
>
> Although the number of different concepts combined here might a bit
> distract from the point.  Also, it'll need steering commitee approval
> to put this code, which was previously contributed under the GPL (on the
> branch) into GFDL documentation.
>
> Or should I make up a reduced/synthetic example for a simpler - but
> probably pointless as an actual output template - example?

Something smaller (but pointless) woudl be nice.

Richard.

Patch

Index: genoutput.c
===================================================================
--- genoutput.c	(revision 191429)
+++ genoutput.c	(working copy)
@@ -662,19 +662,55 @@  process_template (struct data *d, const
      list of assembler code templates, one for each alternative.  */
   else if (template_code[0] == '@')
     {
-      d->template_code = 0;
-      d->output_format = INSN_OUTPUT_FORMAT_MULTI;
+      int found_star = 0;
 
-      printf ("\nstatic const char * const output_%d[] = {\n", d->code_number);
+      for (cp = &template_code[1]; *cp; )
+	{
+	  while (ISSPACE (*cp))
+	    cp++;
+	  if (*cp == '*')
+	    found_star = 1;
+	  while (!IS_VSPACE (*cp) && *cp != '\0')
+	    ++cp;
+	}
+      d->template_code = 0;
+      if (found_star)
+	{
+	  d->output_format = INSN_OUTPUT_FORMAT_FUNCTION;
+	  puts ("\nstatic const char *");
+	  printf ("output_%d (rtx *operands ATTRIBUTE_UNUSED, "
+		  "rtx insn ATTRIBUTE_UNUSED)\n", d->code_number);
+	  puts ("{");
+	  puts ("  switch (which_alternative)\n    {");
+	}
+      else
+	{
+	  d->output_format = INSN_OUTPUT_FORMAT_MULTI;
+	  printf ("\nstatic const char * const output_%d[] = {\n",
+		  d->code_number);
+	}
 
       for (i = 0, cp = &template_code[1]; *cp; )
 	{
-	  const char *ep, *sp;
+	  const char *ep, *sp, *bp;
 
 	  while (ISSPACE (*cp))
 	    cp++;
 
-	  printf ("  \"");
+	  bp = cp;
+	  if (found_star)
+	    {
+	      printf ("    case %d:", i);
+	      if (*cp == '*')
+		{
+		  printf ("\n      ");
+		  cp++;
+		}
+	      else
+		printf (" return \"");
+	    }
+	  else
+	    printf ("  \"");
 
 	  for (ep = sp = cp; !IS_VSPACE (*ep) && *ep != '\0'; ++ep)
 	    if (!ISSPACE (*ep))
@@ -690,7 +726,18 @@  process_template (struct data *d, const
 	      cp++;
 	    }
 
-	  printf ("\",\n");
+	  if (!found_star)
+	    puts ("\",");
+	  else if (*bp != '*')
+	    puts ("\";");
+	  else
+	    {
+	      /* The usual action will end with a return.
+		 If there is neither break or return at the end, this is
+		 assumed to be intentional; this allows to have multiple
+		 consecutive alternatives share some code.  */
+	      puts ("");
+	    }
 	  i++;
 	}
       if (i == 1)
@@ -700,7 +747,10 @@  process_template (struct data *d, const
 	error_with_line (d->lineno,
 			 "wrong number of alternatives in the output template");
 
-      printf ("};\n");
+      if (found_star)
+	puts ("      default: gcc_unreachable ();\n    }\n}");
+      else
+	printf ("};\n");
     }
   else
     {