Re: [PING] genattrab.c generate switch

[Patrick Palka <patrick at parcs dot ath dot cx>]

On Fri, 4 Mar 2016, David Malcolm wrote:

> On Thu, 2016-03-03 at 17:36 -0500, Patrick Palka wrote:
> > On Thu, Mar 3, 2016 at 4:29 PM, Jesper Broge JÃrgensen
> > <jesperbroge@gmail.com> wrote:
> > > On 18/02/16 13:22, Bernd Schmidt wrote:
> > > > On 01/19/2016 12:47 PM, Jesper Broge JÃrgensen wrote:
> > > > > Here is the reformatted patch:
> > > >
> > > >
> > > > This will probably have to wait until stage1.
> > > >
> > > > > +      const int code = GET_CODE (op2);
> > > > > +      if (code != IOR)
> > > > > +    {
> > > > > +      if (code == EQ_ATTR)
> > > >
> > > >
> > > > All the formatting still looks completely mangled. This was
> > > > probably done
> > > > by your mailer. Please try attaching the diff as text/plain.
> > > >
> > > >
> > > > Bernd
> > > Hi i send the patch back as an attatchment as requested about two
> > > weeks ago
> > > (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01256.html) but i
> > > have not
> > > received any response.
> > >
> > > If it has to wait for stage 1 are there anything else i can do for
> > > the patch
> > > untill then?
> >
> > I still suggest to try making write_test_expr() avoid emitting
> > redundant parentheses for chains of || or &&, which would fix the
> > original issue all the same.  Previously you claimed that such a
> > change would not be simpler than your current patch, but I gave it a
> > quick try and ended up with a much smaller patch:
> >
> >  gcc/genattrtab.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
>
> Patrick, did you forget to attach the patch?  I see the diffstat, but
> no patch.

Here it is:

-- >8 --

Subject: [PATCH] Reduce nesting of parentheses in conditionals generated by
 genattrtab

gcc/ChangeLog:

	* genattrtab.c (write_test_expr): New parameter EMIT_PARENS
	which defaults to true.  Emit an outer pair of parentheses only if
	EMIT_PARENS.  When continuing a chain of && or ||, don't emit
	parentheses for the right-hand operand.
---
 gcc/genattrtab.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index b64d8b9..e2ccf1f 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -3432,16 +3432,16 @@ find_attrs_to_cache (rtx exp, bool create)
 #define FLG_OUTSIDE_AND		8

 static unsigned int
-write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags)
+write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
+		 bool emit_parens = true)
 {
   int comparison_operator = 0;
   RTX_CODE code;
   struct attr_desc *attr;

-  /* In order not to worry about operator precedence, surround our part of
-     the expression with parentheses.  */
+  if (emit_parens)
+    fprintf (outf, "(");

-  fprintf (outf, "(");
   code = GET_CODE (exp);
   switch (code)
     {
@@ -3575,8 +3575,18 @@ write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags)
 	      || GET_CODE (XEXP (exp, 1)) == EQ_ATTR
 	      || (GET_CODE (XEXP (exp, 1)) == NOT
 		  && GET_CODE (XEXP (XEXP (exp, 1), 0)) == EQ_ATTR)))
-	attrs_cached
-	  = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags);
+	{
+	  bool need_parens = true;
+
+	  /* No need to emit parentheses around the right-hand operand if we are
+	     continuing a chain of && or ||.  */
+	  if (GET_CODE (XEXP (exp, 1)) == code)
+	    need_parens = false;
+
+	  attrs_cached
+	    = write_test_expr (outf, XEXP (exp, 1), attrs_cached, flags,
+			       need_parens);
+	}
       else
 	write_test_expr (outf, XEXP (exp, 1), attrs_cached,
 			 flags | comparison_operator);
@@ -3794,7 +3804,9 @@ write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags)
 	     GET_RTX_NAME (code));
     }

-  fprintf (outf, ")");
+  if (emit_parens)
+    fprintf (outf, ")");
+
   return attrs_cached;
 }

--
2.8.0.rc0.11.g9bfbc33