[PATCH] resubmitted for review to fix modulo scheduling. COMMITTED

Kenneth Zadeck zadeck@naturalbridge.com
Sat Jun 3 17:41:00 GMT 2006


Bernd,

This patch makes the way that you wish strict_lower_part subreg stores
the default and adds the flag DF_PARTIAL to allow the semantics that
appear to be useful under certain restricted conditions.   I understand
your view that the dataflow information that is produced with DF_PARTIAL
is not obvious, however I believe that I have shown that those semantics
can be useful for at least a few optimizations.  Since the model that df
supports is where one produces dataflow information only for the pass in
which it is used, it seems that there is little harm with allowing this
as an optional knob for the person writing the pass to turn. 

I have bootstrapped and regression tested this patch on the platforms:

x86_64-unknown-linux-gnu
powerpc64-unknown-linux-gnu
i686-pc-linux-gnu

Kenny

2006-03-28  Daniel Berlin  <dberlin@dberlin.org>
        Kenneth Zadeck <zadeck@naturalbridge.com>

        * df.h (DF_PARTIAL): New flag to control generation of
    DF_REF_PARTIAL.
    * df-scan.c (df_ref_record, df_def_record_1, df_uses_record)
    Control the generation of DF_REF_PARTIAL and phantom uses
    for strict_lower_part subregs.



Bernd Schmidt wrote:
> To summarize what we went through on IRC (correct me if I
> misunderstood anything): the current code is in fact a bit
> inconsistent; Kenny's arguments were based on the belief that we did
> not generate uses for partial defs, while in fact we do.  So, the DCE
> example that was posted would in fact not work currently.
>
> Kenny proposed a flag that could be passed to DF to switch between two
> possible behaviours of the DF code.  One would generate both a use and
> a set for a partial access, the DF_REF_PARTIAL bit would become
> superfluous.  The other would not create uses for partial defs, and
> instead mark them specially with DF_REF_PARTIAL, so that users of the
> df information would treat them differently.
>
> Kenneth Zadeck wrote:
>
>> It is not my contention that your way is wrong or inferior.  My
>> contention is that different optimizations will want to have this corner
>> of the problem resolved in different ways:
>
> I could be convinced with a really good example, but so far I'm of the
> opinion that partialness of a set is not a property that is
> interesting: neither for the DCE example, nor for webs.  In both cases
> it is a only a special case of something more general, and mixing up
> the separate concepts is not helpful, and rather a sign of unclear
> thinking.  To see this, always consider the equivalence of the
> following two instructions (slightly changed from last time):
>
>   (set (strict_low (op 0)) (op 1))
>   (set (op 0) (ior (and (match_dup 0) 0xFFFFFF00) (zero_extend (op 1)))
>
> and ask yourself why any pass would want to treat them differently. 
> To recap for DCE: the interesting property is that a use is only used
> to produce a dead def, and therefore doesn't count when determining
> the liveness of its own defs.  Using partialness here is a hack which
> would catch only a tiny fraction of the cases where this situation can
> be detected (how many strict_low_part sets in a parallel with an
> independent other set have you ever seen?).
>
>> As far as web analysis goes, the idea is to split a pseudo into two or
>> more new pseudo if the graphs can be separated.  If the only connection
>> between two components of a graph is thru an instruction that has a
>> subreg set then your way of representing it, as a reference and then a
>> full def would break the optimization. 
>
> If you rely solely on DF_REF_PARTIAL for this, then your optimization
> is too simpleminded - it's already broken since it doesn't take
> match_dup into account.  Again, partialness is not the property you're
> looking for: instead it's the question whether, if you have both a use
> and a def in an insn, you can have different rtl for the two.  A
> partial set is one case where you obviously can't because you have
> only one location for it, but a match_dup is another.  If you're
> looking at this optimization from the perspective of a register
> allocator, then you may also want to consider matching constraints.
>
> The webs that are produced without treating partial defs specially are
> perfectly correct according to the definition of a web.  You just
> can't necessarily use them directly for any given purpose - you may
> have to join multiple webs according to other criteria.  web.c already
> does things like that, e.g.
>   /* A READ_WRITE use requires the corresponding def to be in the same
>      register.  Find it and union.  */
> So, web.c already expects that the df code uses the normal, expected
> definition of defs and uses, and tries to deal with it where it must.
> (I'm not convinced that the bit in that function which allegdly
> handles match_dups actually works.  I don't think df-scan tries to
> look for match_dups and mark them as READ_WRITE refs - it would be
> real nifty if it did.)
>
>>> However, the result of such an exercise is not to proclaim the winner,
>>> it would be to fix the algs so that they are getting the best
>>> information and need the fewest workarounds.
>
> I'm not interesting in finding a winner.  I'm interested in fixing
> code which I believe does highly nonobvious things which are not
> documented and may therefore lead to really nasty surprises when
> anyone tries to use it.  I'll say it more plainly: what we have now is
> broken.
>
> The original topic was the use of DF_REF_PARTIAL in the RU and RD
> problems (possibly others, I haven't looked), where I still believe we
> should remove the special case.  To spell out my argument again in all
> detail:
> 1. These are supposed to be textbook algorithms (at least they use the
>    same names), so if they don't behave as you'd expect after reading a
>    compiler book, then either the algorithm, or its documentation is
>    seriously wrong.
> 2. One possible set of definitions is that a USE of a register is a
>    place where its value is used, and a DEF of a register is a place
>    where its value is modified.  I claim that this is a fairly good
>    definition which would surprise few people.  It's also demonstrably
>    the right definition to use for a fair number of optimization passes,
>    such as (for example) fwprop, which could break if the definition was
>    different.
> 3. If the df code wants to use a different definition where a partial
>    set is not a USE, even though it uses the previous value, or not a
>    DEF, even though it modifies the value, then that needs to be
>    documented very thoroughly.  There also needs to be a really good
>    reason for it.
> 4. If no reasonable example of the usefulness of such nonobvious
>    definitions can be found, then there is no reason for the compiler to
>    implement them at all.  Any such example would have to take into
>    account the example of the two same-semantics instructions I posted
>    above.
>
> The issue needs addressing, either with documentation, or by changing
> the code.  So far, I have seen no evidence of (4).  Hence, I must ask
> you to remove the DF_REF_PARTIAL handling from the places in
> df-problems.c where it would lead to unexpected behaviour.
>
>
> Bernd
>
> PS: Danny, your email is bouncing.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dfcontrol2.diff
Type: text/x-patch
Size: 2414 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20060603/1a22dc04/attachment.bin>


More information about the Gcc-patches mailing list