[PATCH] When possible reverse simple reversible insns in debug info (PR debug/43177)
Richard Guenther
rguenther@suse.de
Tue Mar 2 16:48:00 GMT 2010
On Tue, 2 Mar 2010, Jakub Jelinek wrote:
> Hi!
>
> When var-tracking encounters insns like
> (set (reg:DI %rbx) (plus:DI (reg:DI %rdi) (const_int 1)))
> (set (reg:DI %rdi) (const_int 6))
> where %rdi before this contained a value of some user variable (or even
> the variable itself) and contains no other location that holds that value,
> it gives up, saying that the user variable doesn't have a location once
> %rdi is clobbered. This patch improves it at least for cases where the
> value isn't lost and only one VALUE is needed to compute it. During
> processing of the first insn above we note that the a (value:DI n) doesn't
> live just in %rdi, bust also in (plus:DI (value:DI m) (const_int -1))
> where (value:DI m) is the value that lives in %rbx after the insn.
> We add this only if (value:DI n) is preserved (i.e. tracked by VTA).
> As reversible it considers PLUS/MINUS/XOR with CONST_INT or SYMBOL_REF
> as second argument (or register known to contain CONST_INT or SYMBOL_REF),
> or NOT/NEG and for the time being also SIGN_EXTEND/ZERO_EXTEND (it would be
> great if even cselib special cased SIGN_EXTEND/ZERO_EXTEND, by not flushing
> smaller mode values from the hash table, but that would be much larger and
> riskier change than this). Unfortunately, if we don't want to allow
> incorrect debug info in some (unlikely) corner cases, we can't reverse this
> way say multiplication by 2/4 etc. used for array accesses, as some bits are
> lost.
>
> The loc_cmp change is needed because loc_cmp can now compare stuff like
> (subreg:QI (value:DI 1:1) 0)
> with
> (subreg:QI (value:SI 2:2) 0)
> (only the outer mode matters) and we'd ICE when comparing this.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes some
> guality/vla-1.c failures. Ok for trunk?
Ok.
Thanks,
Richard.
> 2010-03-02 Jakub Jelinek <jakub@redhat.com>
>
> PR debug/43177
> * var-tracking.c (loc_cmp): Don't assert VALUEs have the same mode.
> (VAL_EXPR_HAS_REVERSE): Define.
> (reverse_op): New function.
> (add_stores): For reversible operations add an extra MO_VAL_USE.
>
> * gcc.dg/guality/pr43177.c: New test.
>
> --- gcc/var-tracking.c.jj 2010-03-01 16:31:50.000000000 +0100
> +++ gcc/var-tracking.c 2010-03-02 12:15:36.000000000 +0100
> @@ -2478,7 +2478,10 @@ loc_cmp (rtx x, rtx y)
> {
> if (GET_CODE (y) != VALUE)
> return -1;
> - gcc_assert (GET_MODE (x) == GET_MODE (y));
> + /* Don't assert the modes are the same, that is true only
> + when not recursing. (subreg:QI (value:SI 1:1) 0)
> + and (subreg:QI (value:DI 2:2) 0) can be compared,
> + even when the modes are different. */
> if (canon_value_cmp (x, y))
> return -1;
> else
> @@ -4678,6 +4681,10 @@ count_with_sets (rtx insn, struct cselib
> MO_CLOBBER as well. */
> #define VAL_EXPR_IS_CLOBBERED(x) \
> (RTL_FLAG_CHECK1 ("VAL_EXPR_IS_CLOBBERED", (x), CONCAT)->unchanging)
> +/* Whether the location is a CONCAT of the MO_VAL_SET expression and
> + a reverse operation that should be handled afterwards. */
> +#define VAL_EXPR_HAS_REVERSE(x) \
> + (RTL_FLAG_CHECK1 ("VAL_EXPR_HAS_REVERSE", (x), CONCAT)->return_val)
>
> /* Add uses (register and memory references) LOC which will be tracked
> to VTI (bb)->mos. INSN is instruction which the LOC is part of. */
> @@ -4863,6 +4870,92 @@ add_uses_1 (rtx *x, void *cui)
> for_each_rtx (x, add_uses, cui);
> }
>
> +/* Attempt to reverse the EXPR operation in the debug info. Say for
> + reg1 = reg2 + 6 even when reg2 is no longer live we
> + can express its value as VAL - 6. */
> +
> +static rtx
> +reverse_op (rtx val, const_rtx expr)
> +{
> + rtx src, arg, ret;
> + cselib_val *v;
> + enum rtx_code code;
> +
> + if (GET_CODE (expr) != SET)
> + return NULL_RTX;
> +
> + if (!REG_P (SET_DEST (expr)) || GET_MODE (val) != GET_MODE (SET_DEST (expr)))
> + return NULL_RTX;
> +
> + src = SET_SRC (expr);
> + switch (GET_CODE (src))
> + {
> + case PLUS:
> + case MINUS:
> + case XOR:
> + case NOT:
> + case NEG:
> + case SIGN_EXTEND:
> + case ZERO_EXTEND:
> + break;
> + default:
> + return NULL_RTX;
> + }
> +
> + if (!REG_P (XEXP (src, 0)) || !SCALAR_INT_MODE_P (GET_MODE (src)))
> + return NULL_RTX;
> +
> + v = cselib_lookup (XEXP (src, 0), GET_MODE (XEXP (src, 0)), 0);
> + if (!v || !cselib_preserved_value_p (v))
> + return NULL_RTX;
> +
> + switch (GET_CODE (src))
> + {
> + case NOT:
> + case NEG:
> + if (GET_MODE (v->val_rtx) != GET_MODE (val))
> + return NULL_RTX;
> + ret = gen_rtx_fmt_e (GET_CODE (src), GET_MODE (val), val);
> + break;
> + case SIGN_EXTEND:
> + case ZERO_EXTEND:
> + ret = gen_lowpart_SUBREG (GET_MODE (v->val_rtx), val);
> + break;
> + case XOR:
> + code = XOR;
> + goto binary;
> + case PLUS:
> + code = MINUS;
> + goto binary;
> + case MINUS:
> + code = PLUS;
> + goto binary;
> + binary:
> + if (GET_MODE (v->val_rtx) != GET_MODE (val))
> + return NULL_RTX;
> + arg = XEXP (src, 1);
> + if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)
> + {
> + arg = cselib_expand_value_rtx (arg, scratch_regs, 5);
> + if (arg == NULL_RTX)
> + return NULL_RTX;
> + if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)
> + return NULL_RTX;
> + }
> + ret = simplify_gen_binary (code, GET_MODE (val), val, arg);
> + if (ret == val)
> + /* Ensure ret isn't VALUE itself (which can happen e.g. for
> + (plus (reg1) (reg2)) when reg2 is known to be 0), as that
> + breaks a lot of routines during var-tracking. */
> + ret = gen_rtx_fmt_ee (PLUS, GET_MODE (val), val, const0_rtx);
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +
> + return gen_rtx_CONCAT (GET_MODE (v->val_rtx), v->val_rtx, ret);
> +}
> +
> /* Add stores (register and memory references) LOC which will be tracked
> to VTI (bb)->mos. EXPR is the RTL expression containing the store.
> CUIP->insn is instruction which the LOC is part of. */
> @@ -4879,6 +4972,7 @@ add_stores (rtx loc, const_rtx expr, voi
> bool track_p = false;
> cselib_val *v;
> bool resolve, preserve;
> + rtx reverse;
>
> if (type == MO_CLOBBER)
> return;
> @@ -5082,6 +5176,16 @@ add_stores (rtx loc, const_rtx expr, voi
>
> */
>
> + if (GET_CODE (PATTERN (cui->insn)) != COND_EXEC)
> + {
> + reverse = reverse_op (v->val_rtx, expr);
> + if (reverse)
> + {
> + loc = gen_rtx_CONCAT (GET_MODE (mo->u.loc), loc, reverse);
> + VAL_EXPR_HAS_REVERSE (loc) = 1;
> + }
> + }
> +
> mo->u.loc = loc;
>
> if (track_p)
> @@ -5367,10 +5471,17 @@ compute_bb_dataflow (basic_block bb)
> case MO_VAL_SET:
> {
> rtx loc = VTI (bb)->mos[i].u.loc;
> - rtx val, vloc, uloc;
> + rtx val, vloc, uloc, reverse = NULL_RTX;
>
> - vloc = uloc = XEXP (loc, 1);
> - val = XEXP (loc, 0);
> + vloc = loc;
> + if (VAL_EXPR_HAS_REVERSE (loc))
> + {
> + reverse = XEXP (loc, 1);
> + vloc = XEXP (loc, 0);
> + }
> + uloc = XEXP (vloc, 1);
> + val = XEXP (vloc, 0);
> + vloc = uloc;
>
> if (GET_CODE (val) == CONCAT)
> {
> @@ -5443,6 +5554,10 @@ compute_bb_dataflow (basic_block bb)
> var_regno_delete (out, REGNO (uloc));
>
> val_store (out, val, vloc, insn, true);
> +
> + if (reverse)
> + val_store (out, XEXP (reverse, 0), XEXP (reverse, 1),
> + insn, false);
> }
> break;
>
> @@ -7012,10 +7127,17 @@ emit_notes_in_bb (basic_block bb, datafl
> case MO_VAL_SET:
> {
> rtx loc = VTI (bb)->mos[i].u.loc;
> - rtx val, vloc, uloc;
> + rtx val, vloc, uloc, reverse = NULL_RTX;
>
> - vloc = uloc = XEXP (loc, 1);
> - val = XEXP (loc, 0);
> + vloc = loc;
> + if (VAL_EXPR_HAS_REVERSE (loc))
> + {
> + reverse = XEXP (loc, 1);
> + vloc = XEXP (loc, 0);
> + }
> + uloc = XEXP (vloc, 1);
> + val = XEXP (vloc, 0);
> + vloc = uloc;
>
> if (GET_CODE (val) == CONCAT)
> {
> @@ -7083,6 +7205,10 @@ emit_notes_in_bb (basic_block bb, datafl
>
> val_store (set, val, vloc, insn, true);
>
> + if (reverse)
> + val_store (set, XEXP (reverse, 0), XEXP (reverse, 1),
> + insn, false);
> +
> emit_notes_for_changes (NEXT_INSN (insn), EMIT_NOTE_BEFORE_INSN,
> set->vars);
> }
> --- gcc/testsuite/gcc.dg/guality/pr43177.c.jj 2010-03-01 16:36:20.000000000 +0100
> +++ gcc/testsuite/gcc.dg/guality/pr43177.c 2010-03-01 16:36:20.000000000 +0100
> @@ -0,0 +1,36 @@
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +void __attribute__((noinline))
> +bar (long x)
> +{
> + asm volatile ("" : : "r" (x) : "memory");
> +}
> +
> +long __attribute__((noinline))
> +foo (long x)
> +{
> + long l = x + 3;
> + bar (l);/* { dg-final { gdb-test 15 "l" "10" } } */
> + bar (l);/* { dg-final { gdb-test 15 "x" "7" } } */
> + return l;
> +}
> +
> +long __attribute__((noinline))
> +baz (int x)
> +{
> + long l = x + 3;
> + bar (l);/* { dg-final { gdb-test 24 "l" "10" } } */
> + bar (l);/* { dg-final { gdb-test 24 "x" "7" } } */
> + return l;
> +}
> +
> +int
> +main (void)
> +{
> + int i;
> + asm volatile ("" : "=r" (i) : "0" (7));
> + foo (i);
> + baz (i);
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
More information about the Gcc-patches
mailing list