Reload bug & SRA oddness

[Richard Guenther]

On 4/28/07, Daniel Berlin <dberlin@dberlin.org> wrote:
> On 4/28/07, Richard Guenther <richard.guenther@gmail.com> wrote:
> > On 4/28/07, Bernd Schmidt <bernds_cb1@t-online.de> wrote:
> > > Daniel Berlin wrote:
> > > > It would also be a mistake to try to do significant optimization of
> > > > copy in/out placement directly in SRA.  Again, that is why we have PRE
> > > > and DSE.  If they are not moving/eliminating these things, they need
> > > > to be fixed.
> > >
> > > I've had a look at why these statements are not eliminated.  I figure
> > > that dce is the pass that should get rid of them, but it doesn't.  From
> > > the dumps:
> > >
> > >    # SFT.70_14 = VDEF <SFT.70_13(D)>
> > >    y.k = y$k_9(D);
> > >    # VUSE <sD_15(D)>
> > >    # SFT.70_19 = VDEF <SFT.70_14>
> > >    # SFT.71_20 = VDEF <SFT.71_16(D)>
> > >    # SFT.72_21 = VDEF <SFT.72_17(D)>
> > >    # SFT.73_22 = VDEF <SFT.73_18(D)>
> > >    y = sD;
> > >
> > > Note how all the VDEFs have an input argument, which causes us to
> > > believe that the second statement depends on the first.  AFAICT, these
> > > VDEFs should have no arguments, but the compiler always generates them
> > > with exactly one input.  I've tried to fix that, but so far the patch I
> > > have is only useful as an SSA learning tool for myself, as it creates
> > > lots of failures elsewhere in the compiler...
> > >
> > > Any other ideas how to address this?
> >
> > These are all the default SSA_NAMEs of the SFTs (note the (D)).  If
> > y is a local variable DCE should "ignore" them (those uses are undefined
> > in that case).  So the only relevant VDEF for DCE/DSE is
> >
> > # SFT.70_19 = VDEF <SFT.70_14>
> >
> Right.
>
> I would imagine *DSE* should eliminate this, not DCE, because it's
> store over store.
> However, this does show a problem with our SSA form, actually.
> Our VDEF's treat the RHS argument of the VDEF as an implicit use (and
> will avoid adding explicit vuses of those arguments), besides just the
> next part of the def-def chain to follow.  However, in cases like the
> above, there *is no use of SFT.70_14* in that statement, only a def.
> We don't have a way to specify that, so DCE will mark the statement
> defining SFT.70_14 as necessary.
>
> DSE should do better, since it was taught how to deal with this
> problem by seeing if the store is to the same place, or subsumed by
> the later store with no intervening uses.

Now the problem with DSE seems to be that for

(gdb) call debug_generic_expr (stmt)
# SFT.84_24 = VDEF <SFT.84_10(D)> { SFT.84 }
y.k = y$k_23(D)
(gdb) call debug_generic_expr (use_stmt)
# VUSE <sD_9(D)> { sD }
# SFT.84_14 = VDEF <SFT.84_24>
# SFT.85_15 = VDEF <SFT.85_11(D)>
# SFT.86_16 = VDEF <SFT.86_12(D)>
# SFT.87_17 = VDEF <SFT.87_13(D)> { SFT.84 SFT.85 SFT.86 SFT.87 }
y = sD

at tree-ssa-dse.c:604 (the call to dse_partial_kill_p):

      /* If we have precisely one immediate use at this point, then we may
         have found redundant store.  Make sure that the stores are to
         the same memory location.  This includes checking that any
         SSA-form variables in the address will have the same values.  */
      if (use_p != NULL_USE_OPERAND_P
          && bitmap_bit_p (dse_gd->stores, get_stmt_uid (use_stmt))
          && (operand_equal_p (GIMPLE_STMT_OPERAND (stmt, 0),
                               GIMPLE_STMT_OPERAND (use_stmt, 0), 0)
              || dse_partial_kill_p (stmt, dse_gd))
          && memory_address_same (stmt, use_stmt))

dse_partial_kill_p does not do what its same suggests, but first asserts
that stmt stores to all of the aggregate (which it of course does not).
It _looks_ like that maybe dse_partial_kill_p (use_stmt, dse_gd) was
intended here.  Otherwise DSE detects stmt as possible dead store.

Richard.