Re: Bug 14562 : copyrename & PRE

[Daniel Berlin <dberlin at dberlin dot org>]

 On Mar 15, 2004, at 10:36 AM, Andrew MacLeod wrote:

 >
 > copyrename doesnt *appear* to be doing anything wrong.
 >
 > The original program had:
 >
 >   T.1<D1071>_29 = k<D1063>_2 - h<D1062>_5;
 >   T.2<D1072>_30 = T.1<D1071>_29 * 4;
 >
 > and copy rename decided that version _29 would be better off as a 'k',
 > so it produces:>
 >
 >   k<D1063>_29 = k<D1063>_2 - h<D1062>_5;
 >   T.2<D1072>_30 = k<D1063>_29 * 4;
 >
 > and PRE decides that worth doing something with:
 >
 >   k<D1063>_29 = k<D1063>_2 - h<D1062>_5;
 >   pretmp.18<D1129>_50 = k<D1063>_29 * 4;
 >   T.2<D1072>_30 = pretmp.18<D1129>_50;
 >
 > In the -fno-tree-copyrename version, PRE doesn't touch T.1_29... but it
 > does when it becomes 'k'.
 >
 Because PARM_DECL's don't have a bb_for_stmt(SSA_NAME_DEF_STMT
 (PARM_DECL)), we special case them in PRE when it comes to seeing if
 they are actually undefined or not.

 In reality, they should have a bb_for_stmt (SSA_NAME_DEF_STMT
 (parm_decl)) of ENTRY_BLOCK_PTR for those live on entry, and proper
 bb_for_stmt (SSA_NAME_DEF_STMT (parm_decl)) for others.

 People seemed to agree with this (Richard and Jeff, IIRC), i just never
 got around to it.

 The code in question that treats PARM_DECL special is:


    /* This guards against moving around undefined variables.
     However, PARM_DECL is special because it *IS* live on entry,
     so it's not really undefined.  */
        if (!defbb && TREE_CODE (SSA_NAME_VAR (use)) != PARM_DECL)
    return true;
        else if (!defbb && TREE_CODE (SSA_NAME_VAR (use)) == PARM_DECL)
          return false;
        if (dominated_by_p (CDI_DOMINATORS, bb, defbb))
    return false;


 Does copyrename make this no longer true somehow?
 If so, we probably should just make the bb_for_stmt (SSA_NAME_DEF_STMT
 (PARM_DECL)) be ENTRY_BLOCK_PTR when it is approriate, and stop special
 casing them, which is probably causing this bug.

 This is the only place we special case PARM_DECL in all of PRE.

 There is a test in the tree-ssa testsuite to make sure we perform
 SSAPRE PARM_DECL's properly that was committed when the above
 workaround was committed.

 > Do we have magic rules about PARMs where they aren't allowed to be
 > assign to? 'k' is a PARM, and PRE appears to be make an assumption
 > about
 > it. Up until now we haven't really been seeing a DEF of a PARM, its
 > always been into a temp variable.
 >
 > Is there something about PARMs that I ought not allow a rename on the
 > LHS to a PARM, or is PRE making a bad assumption?
 >
 > Andrew
 >