This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix PR rtl-optimization/54870


On Sun, Oct 14, 2012 at 10:47 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> This is the execution failure of gfortran.dg/array_constructor_4.f90 in 64-bit
> mode on SPARC/Solaris at -O3.  The dse2 dump for the reduced testcase reads:
>
> dse: local deletions = 0, global deletions = 1, spill deletions = 0
> starting the processing of deferred insns
> deleting insn with uid = 25.
> ending the processing of deferred insns
>
> but the memory location stored to:
>
> (insn 25 27 154 2 (set (mem/c:SI (plus:DI (reg/f:DI 30 %fp)
>                 (const_int 2039 [0x7f7])) [6 A.1+16 S4 A64])
>         (reg:SI 1 %g1 [136])) array_constructor_4.f90:4 61 {*movsi_insn}
>      (nil))
>
> is read by a subsequent call to memcpy.
>
> It turns out that this memcpy call is generated for an aggregate assignment:
>
>   MEM[(c_char * {ref-all})&i] = MEM[(c_char * {ref-all})&A.17];
>
> Note the A.1 in the store and the A.17 in the load. A.1 and A.17 are aggregate
> variables sharing the same stack slot.  A.17 is correcty marked as addressable
> because of the call to memcpy, but A.1 isn't since its address isn't taken,
> and DSE can optimize away (since 4.7) stores if their MEM_EXPR doesn't escape.
>
> The store is reaching the load because an intermediate store into A.17:
>
> (insn 78 76 82 6 (set (mem/c:SI (plus:DI (reg/f:DI 30 %fp)
>                 (const_int 2039 [0x7f7])) [6 A.17+16 S4 A64])
>         (reg:SI 1 %g1 [136])) array_constructor_4.f90:14 61 {*movsi_insn}
>      (nil))
>
> has been deleted by postreload as no-op (because redundant), thus making A.1
> partially escape without marking it as addressable.
>
> The attached patch uses cfun->gimple_df->escaped.vars to plug the hole: when
> mark_addressable is called during RTL expansion and the decl is partitioned,
> all the variables in the partition are added to the bitmap.  Then can_escape
> is changed to additionally test cfun->gimple_df->escaped.vars.
>
> Tested on x86-64/Linux and SPARC64/Solaris, OK for mainline and 4.7 branch?

Hmm.  I think this points to an issue with update_alias_info_with_stack_vars
instead.  That is, this function should have already cared for handling this
case where two decls have their stack slot shared.  What it seems to get
confused about is addressability, or rather can_escape is not using the
RTL alias export properly.  Instead of

static bool
can_escape (tree expr)
{
  tree base;
  if (!expr)
    return true;
  base = get_base_address (expr);
  if (DECL_P (base)
      && !may_be_aliased (base))
    return false;
  return true;

it needs to check decls_to_pointers[base] and then check
if any of the pointed-to decls may be aliased.

Now, that's not that easy because we don't have a
mapping from DECL UID to DECL (and the decl
isn't in the escaped solution if it is just used by
memcpy), but we could compute a bitmap of
all address-taken decls in update_alias_info_with_stack_vars
or simply treat all check decls_to_pointers[base] != NULL
bases as possibly having their address taken.

Richard.

>
> 2012-10-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         PR rtl-optimization/54870
>         * dse.c (can_escape): Test cfun->gimple_df->escaped.vars as well.
>         * gimplify.c (mark_addressable): If this is a partition decl, add
>         all the variables in the partition to cfun->gimple_df->escaped.vars.
>
>
> --
> Eric Botcazou


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]