On Fri, 4 Mar 2016, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 04:13:41PM +0100, Bernd Schmidt wrote:
> > On 03/04/2016 03:27 PM, Patrick Palka wrote:
> > >>>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:
> >
> > This looks like a reasonable stopgap if a release manager thinks this is
> > important enough to fix for gcc-6. Some comments below for that case. Longer
>
> I think it is important for gcc-6, because one compiler for
> weirdo reasons imposes unreasonably small restrictions on the () nesting
> and some people use it as stage1 compiler.
>
> So, with the formatting nits fixed, if it got appropriately tested, I think
> we want it for stage4.
I updated the function comment and made sure that the indentation is
correct. Earlier I successfully bootstrapped + tested this patch on
x86_64-pc-linux-gnu, but I will do so again to make sure. This patch
reduces the maximum parentheses nesting level in insn-attrtab.c on
x86_64 from about 31 to about 6.
OK to commit after testing?
-- >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 | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index b64d8b9..5974f3e 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -3416,7 +3416,10 @@ find_attrs_to_cache (rtx exp, bool create)
/* Given a piece of RTX, print a C expression to test its truth value to OUTF.
We use AND and IOR both for logical and bit-wise operations, so
- interpret them as logical unless they are inside a comparison expression.
*/
+ interpret them as logical unless they are inside a comparison expression.
+
+ An outermost pair of parentheses is emitted around this C expression unless
+ EMIT_PARENS is false. */
/* Interpret AND/IOR as bit-wise operations instead of logical. */
#define FLG_BITWISE 1
@@ -3432,16 +3435,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 +3578,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 +3807,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
