PR debug/60655, debug loc expressions
Richard Biener
richard.guenther@gmail.com
Tue Sep 9 14:25:00 GMT 2014
On Tue, Sep 9, 2014 at 1:50 PM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Sep 05, 2014 at 11:00:04AM +0930, Alan Modra wrote:
>> Of course it would be better to repair the damage done to debug info
>> rather than rejecting it outright..
>
> This cures PR60655 on PowerPC by passing the horrible debug_loc
> expressions we have through simplify_rtx. Not only do we get
> reg10 + &.LANCHOR0 + const(0x14f - &.LANCHOR0) and
> reg10 + &modulus + const(~&d_data),
> the two expressions that cause the PR due to (CONST (MINUS ..)) and
> (CONST (NOT ..)), but also things like
> ~reg5 - reg31 + reg5 + reg10 + &modulus
> where "~reg5 + reg5" is an inefficient way to write "-1".
>
> It turns out that merely passing these expression through simplify_rtx
> isn't enough. Some tweaking is necessary to make (CONST (NOT ..)) and
> (CONST (MINUS ..)) recognised by simplify_plus_minus, and even after
> doing that, the two reg5 terms are not cancelled.
>
> The reg5 case starts off with the simplify_plus_minus sorted ops array
> effectively containing the expression
> -reg31 + reg10 + reg5 + -reg5 + &modulus + -1
>
> The first combination tried is &modulus + -1, which is rejected to
> prevent recursion. The next combination tried is -reg5 + -1, which is
> simlified to ~reg5. Well, that is a valid simplification, but the
> trouble is that this prevents "reg5 + -reg5" being simplified to 0.
> What's more, "&modulus + -1" is no more expensive than "&modulus",
> since they are both emitted as an address field with a symbol+addend
> relocation. For that reason, I believe we should not consider
> combining a const_int with any other term after finding that it can be
> combined with a symbol_ref or other similar term.
>
> Bootstrapped and regression tested powerpc64-linux and x86_64-linux.
> I measured before/after bootstrap times on x86_64-linux because I was
> concerned about the extra simplify_rtx calls, and was pleasantly
> surprised to see a 0.23% improvement in bootstrap time. (Which of
> course is likely just noise.) OK to apply?
>
> PR debug/60655
> * simplify-rtx.c (simplify_plus_minus): Delete unused "input_ops".
> Handle CONST wrapped NOT, NEG and MINUS. Break out of innermost
> loop when finding a trivial CONST expression.
> * var-tracking.c (vt_expand_var_loc_chain): Call simplify_rtx.
>
> Index: gcc/simplify-rtx.c
> ===================================================================
> --- gcc/simplify-rtx.c (revision 214487)
> +++ gcc/simplify-rtx.c (working copy)
> @@ -3960,7 +3960,7 @@ simplify_plus_minus (enum rtx_code code, enum mach
> {
> struct simplify_plus_minus_op_data ops[8];
> rtx result, tem;
> - int n_ops = 2, input_ops = 2;
> + int n_ops = 2;
> int changed, n_constants = 0, canonicalized = 0;
> int i, j;
>
> @@ -3997,7 +3997,6 @@ simplify_plus_minus (enum rtx_code code, enum mach
> n_ops++;
>
> ops[i].op = XEXP (this_op, 0);
> - input_ops++;
> changed = 1;
> canonicalized |= this_neg;
> break;
> @@ -4010,14 +4009,35 @@ simplify_plus_minus (enum rtx_code code, enum mach
> break;
>
> case CONST:
> - if (n_ops < 7
> - && GET_CODE (XEXP (this_op, 0)) == PLUS
> - && CONSTANT_P (XEXP (XEXP (this_op, 0), 0))
> - && CONSTANT_P (XEXP (XEXP (this_op, 0), 1)))
> + if (GET_CODE (XEXP (this_op, 0)) == NEG
> + && CONSTANT_P (XEXP (XEXP (this_op, 0), 0)))
> {
> ops[i].op = XEXP (XEXP (this_op, 0), 0);
> + ops[i].neg = !this_neg;
> + changed = 1;
> + canonicalized = 1;
> + }
> + else if (n_ops < 7
> + && GET_CODE (XEXP (this_op, 0)) == NOT
> + && CONSTANT_P (XEXP (XEXP (this_op, 0), 0)))
> + {
> + ops[n_ops].op = CONSTM1_RTX (mode);
> + ops[n_ops++].neg = this_neg;
> + ops[i].op = XEXP (XEXP (this_op, 0), 0);
> + ops[i].neg = !this_neg;
> + changed = 1;
> + canonicalized = 1;
> + }
> + else if (n_ops < 7
> + && (GET_CODE (XEXP (this_op, 0)) == PLUS
> + || GET_CODE (XEXP (this_op, 0)) == MINUS)
> + && CONSTANT_P (XEXP (XEXP (this_op, 0), 0))
> + && CONSTANT_P (XEXP (XEXP (this_op, 0), 1)))
> + {
> + ops[i].op = XEXP (XEXP (this_op, 0), 0);
> ops[n_ops].op = XEXP (XEXP (this_op, 0), 1);
> - ops[n_ops].neg = this_neg;
> + ops[n_ops].neg
> + = (GET_CODE (XEXP (this_op, 0)) == MINUS) ^ this_neg;
> n_ops++;
> changed = 1;
> canonicalized = 1;
> @@ -4141,16 +4161,21 @@ simplify_plus_minus (enum rtx_code code, enum mach
> else
> tem = simplify_binary_operation (ncode, mode, lhs, rhs);
>
> - /* Reject "simplifications" that just wrap the two
> - arguments in a CONST. Failure to do so can result
> - in infinite recursion with simplify_binary_operation
> - when it calls us to simplify CONST operations. */
> - if (tem
> - && ! (GET_CODE (tem) == CONST
> - && GET_CODE (XEXP (tem, 0)) == ncode
> - && XEXP (XEXP (tem, 0), 0) == lhs
> - && XEXP (XEXP (tem, 0), 1) == rhs))
> + if (tem)
> {
> + /* Reject "simplifications" that just wrap the two
> + arguments in a CONST. Failure to do so can result
> + in infinite recursion with simplify_binary_operation
> + when it calls us to simplify CONST operations.
> + Also, if we find such a simplification, don't try
> + any more combinations with this rhs: We must have
> + something like symbol+offset, ie. one of the
> + trivial CONST expressions we handle later. */
> + if (GET_CODE (tem) == CONST
> + && GET_CODE (XEXP (tem, 0)) == ncode
> + && XEXP (XEXP (tem, 0), 0) == lhs
> + && XEXP (XEXP (tem, 0), 1) == rhs)
> + break;
> lneg &= rneg;
> if (GET_CODE (tem) == NEG)
> tem = XEXP (tem, 0), lneg = !lneg;
> Index: gcc/var-tracking.c
> ===================================================================
> --- gcc/var-tracking.c (revision 214898)
> +++ gcc/var-tracking.c (working copy)
> @@ -8375,7 +8375,15 @@ vt_expand_var_loc_chain (variable var, bitmap regs
> *pendrecp = pending_recursion;
>
> if (!pendrecp || !pending_recursion)
> - var->var_part[0].cur_loc = result;
> + {
> + if (result)
> + {
> + rtx tem = simplify_rtx (result);
why wasn't 'result' built using simplify_gen_* in the first place? I also
note that debug_insns can have all sorts of weird (even invalid) and
un-recognized RTL which the simplify_rtx machinery may not like
(and thus ICE).
CSELIB seems to do the less optimal copy_rtx, valueize operands,
re-simplify operation instead of valueizing the operands and then
simplify_gen_ the actual RTX here (skipping that when valueizing
the operands did nothing).
Richard.
> + if (tem)
> + result = tem;
> + }
> + var->var_part[0].cur_loc = result;
> + }
>
> return result;
> }
>
> --
> Alan Modra
> Australia Development Lab, IBM
More information about the Gcc-patches
mailing list