[PATCH] Fix up location and block handling during expansion (PR debug/19192)

Richard Guenther richard.guenther@gmail.com
Wed Mar 24 10:49:00 GMT 2010


On Wed, Mar 24, 2010 at 11:01 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Attached are two alternative patches to fix PR19192.
> The problem there is when expanding a SSA_NAME, we don't switch to the
> def_stmt's gimple_location and gimple_block, so everything is emitted
> in the scope and with location of the currently expanded stmt.
> After we are done with the def_stmt expansion, we should switch back
> the previous location/block, as further instructions emitted aren't
> from the def_stmt, but from the current stmt.
>
> The first version is probably slightly more riskier, as it affects more
> places (expand_expr/expand_normal/expand_expr_real is called in lots of
> places and it now newly restores the original block and location before the
> call).
>
> Both patches have been bootstrapped/regtested on x86_64-linux and
> i686-linux, both just caused a regression in dwarf2/inline2.c (that's why
> it is included in the patch too).  The difference between unpatched
> and patched gcc is in locations (and as none of the third routine locations
> are current locations of any insns after the patch, its
> DW_TAG_inlined_subroutine/DW_TAG_lexical_block vanishes too):
> -       # inline2.c:45
> -       .loc 1 45 0
> +       # inline2.c:52
> +       .loc 1 52 0
>        movl    8(%ebp), %eax
>        addl    $1, %eax
>        movl    %eax, 0
> -.LBE15:
> -.LBE14:
>        # inline2.c:53
>
> Both of the locations are somewhat right and somewhat wrong.
> The addition is inline2.c:52, while the actual store is inline2.c:45,
> so best would be:
>        # inline2.c:45
>        .loc 1 45 0
>        movl    8(%ebp), %eax
>        addl    $1, %eax
>        # inline2.c:52
>        .loc 1 52 0
>        movl    %eax, 0
>        # inline2.c:53
> The reason for this is that the store is expanded by setting target
> to (mem:SI (reg)) and then expanding the PLUS_EXPR with that target.
> Without the patch, we don't switch expansion for the PLUS_EXPR recursion,
> with the patch the PLUS_EXPR expansion is done with inline2.c:52.
> But as target argument is passed, already the PLUS_EXPR expansion stores
> the result into target and returns target - thus for the inline2.c:45
> location there is no code to be emitted.  Not sure what exactly would
> be needed to make this more exact.
>
> The new testcase fails on i686-linux at -O1 and above, due to a separate
> issue for which I'm going to submit a patch once I bootstrap/regtest it.
>
> Ok for trunk?  Which version?

The first version is ok.

Thanks,
Richard.

>        Jakub
>



More information about the Gcc-patches mailing list