Re: [PATCH] Remove --enable-checking=rtl var-tracking verification (PR debug/46867)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Dec 10, 2010 at 8:48 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As the testcase below shows, even the strip_pointer_flags hack can't
> cure in some cases the differences caused by REG_POINTER vs. lack thereof,
> as simplify-rtx.c doesn't try to swap anything if both operands have the
> same precedence.
>
> The following patch just nukes the extra checking, other alternative would
> be only to hack up swap_commutative_operands_p etc. to consider some
> global flag which var-tracking.c would set and which would say that
> REG_POINTER/MEM_POINTER should be disregarded.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux (with rtl checking),
> ok for trunk?

Ok.

Thanks,
Richard.

> 2010-12-10 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?PR debug/46867
> ? ? ? ?* var-tracking.c (emitted_notes, string_pointer_flags): Removed.
> ? ? ? ?(emit_note_insn_var_location): Remove ENABLE_RTL_CHECKING verification.
> ? ? ? ?(vt_emit_notes): Don't initialize and destroy emitted_notes.
>
> ? ? ? ?* gcc.dg/pr46867.c: New test.
>
> --- gcc/var-tracking.c.jj ? ? ? 2010-12-02 11:51:32.000000000 +0100
> +++ gcc/var-tracking.c ?2010-12-10 15:16:06.000000000 +0100
> @@ -7095,24 +7095,6 @@ vt_expand_loc_dummy (rtx loc, htab_t var
> ? return ret;
> ?}
>
> -#ifdef ENABLE_RTL_CHECKING
> -/* Used to verify that cur_loc_changed updating is safe. ?*/
> -static struct pointer_map_t *emitted_notes;
> -
> -/* Strip REG_POINTER from REGs and MEM_POINTER from MEMs in order to
> - ? avoid differences in commutative operand simplification. ?*/
> -static rtx
> -strip_pointer_flags (rtx x, const_rtx old_rtx ATTRIBUTE_UNUSED,
> - ? ? ? ? ? ? ? ? ? ?void *data ATTRIBUTE_UNUSED)
> -{
> - ?if (REG_P (x) && REG_POINTER (x))
> - ? ?return gen_rtx_REG (GET_MODE (x), REGNO (x));
> - ?if (MEM_P (x) && MEM_POINTER (x))
> - ? ?return gen_rtx_MEM (GET_MODE (x), XEXP (x, 0));
> - ?return NULL_RTX;
> -}
> -#endif
> -
> ?/* Emit the NOTE_INSN_VAR_LOCATION for variable *VARP. ?DATA contains
> ? ?additional parameters: WHERE specifies whether the note shall be emitted
> ? ?before or after instruction INSN. ?*/
> @@ -7157,10 +7139,8 @@ emit_note_insn_var_location (void **varp
> ? ? ? if (var->n_var_parts == 0)
> ? ? ? ?var->cur_loc_changed = true;
> ? ? }
> -#ifndef ENABLE_RTL_CHECKING
> ? if (!var->cur_loc_changed)
> ? ? goto clear;
> -#endif
> ? for (i = 0; i < var->n_var_parts; i++)
> ? ? {
> ? ? ? enum machine_mode mode, wider_mode;
> @@ -7302,36 +7282,6 @@ emit_note_insn_var_location (void **varp
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?parallel, (int) initialized);
> ? ? }
>
> -#ifdef ENABLE_RTL_CHECKING
> - ?if (note_vl)
> - ? ?{
> - ? ? ?void **note_slot = pointer_map_insert (emitted_notes, decl);
> - ? ? ?rtx pnote = (rtx) *note_slot;
> - ? ? ?if (!var->cur_loc_changed && (pnote || PAT_VAR_LOCATION_LOC (note_vl)))
> - ? ? ? {
> - ? ? ? ? rtx old_vl, new_vl;
> - ? ? ? ? gcc_assert (pnote);
> - ? ? ? ? old_vl = PAT_VAR_LOCATION_LOC (pnote);
> - ? ? ? ? new_vl = PAT_VAR_LOCATION_LOC (note_vl);
> - ? ? ? ? if (!rtx_equal_p (old_vl, new_vl))
> - ? ? ? ? ? {
> - ? ? ? ? ? ? /* There might be differences caused by REG_POINTER
> - ? ? ? ? ? ? ? ?differences. ?REG_POINTER affects
> - ? ? ? ? ? ? ? ?swap_commutative_operands_p. ?*/
> - ? ? ? ? ? ? old_vl = simplify_replace_fn_rtx (old_vl, NULL_RTX,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? strip_pointer_flags, NULL);
> - ? ? ? ? ? ? new_vl = simplify_replace_fn_rtx (new_vl, NULL_RTX,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? strip_pointer_flags, NULL);
> - ? ? ? ? ? ? gcc_assert (rtx_equal_p (old_vl, new_vl));
> - ? ? ? ? ? ? PAT_VAR_LOCATION_LOC (note_vl) = new_vl;
> - ? ? ? ? ? }
> - ? ? ? }
> - ? ? ?*note_slot = (void *) note_vl;
> - ? ?}
> - ?if (!var->cur_loc_changed)
> - ? ?goto clear;
> -#endif
> -
> ? if (where != EMIT_NOTE_BEFORE_INSN)
> ? ? {
> ? ? ? note = emit_note_after (NOTE_INSN_VAR_LOCATION, insn);
> @@ -7960,9 +7910,6 @@ vt_emit_notes (void)
> ? basic_block bb;
> ? dataflow_set cur;
>
> -#ifdef ENABLE_RTL_CHECKING
> - ?emitted_notes = pointer_map_create ();
> -#endif
> ? gcc_assert (!htab_elements (changed_variables));
>
> ? /* Free memory occupied by the out hash tables, as they aren't used
> @@ -8022,9 +7969,6 @@ vt_emit_notes (void)
> ? ? ? VEC_free (rtx, heap, changed_values_stack);
> ? ? }
>
> -#ifdef ENABLE_RTL_CHECKING
> - ?pointer_map_destroy (emitted_notes);
> -#endif
> ? emit_notes = false;
> ?}
>
> --- gcc/testsuite/gcc.dg/pr46867.c.jj ? 2010-12-10 16:21:14.000000000 +0100
> +++ gcc/testsuite/gcc.dg/pr46867.c ? ? ?2010-12-10 15:20:53.000000000 +0100
> @@ -0,0 +1,32 @@
> +/* PR debug/46867 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -g" } */
> +
> +typedef __PTRDIFF_TYPE__ ptrdiff_t;
> +
> +extern void *bar (void);
> +
> +void
> +foo (int x, char *r1, char *r2, __INTPTR_TYPE__ *rp, char *t)
> +{
> + ?char *tx = tx;
> + ?char **cc = bar ();
> + ?ptrdiff_t rx = r1 - r2;
> + ?for (;;)
> + ? ?{
> + ? ? ?char *cp1 = (char *) *rp;
> + ? ? ?char *cp2 = t;
> + ? ? ?char *s;
> + ? ? ?ptrdiff_t len = cp1 - tx + rx * (cp2 - cp1);
> + ? ? ?while (len)
> + ? ? ? ;
> + ? ? ?for (s = tx; s; s++)
> + ? ? ? ;
> + ? ? ?while (s)
> + ? ? ? if (s == cc[s - r1])
> + ? ? ? ? if (x)
> + ? ? ? ? ? bar ();
> + ? ? ?if (cp1)
> + ? ? ? tx = cp2;
> + ? ?}
> +}
>
> ? ? ? ?Jakub
>