This is the mail archive of the gcc@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: COND_EXPR lowering patch


Hello,

>  >>  >as the code works now all statements here including the condition
>  >>  >will be marked as useful.
>  >> And we care because? 
>  >
>  >because why would we want to have a code that does not anything at all?
> A later pass will trivially clean this up.  I don't see a need to work hard
> to remove it or to worry about it all because I already know it will go
> away.

I speak about code in gcc.

>  >> Yes, this kid of thing does happen, but not that often.
>  >
>  >No, it will happen always -- you cannot create a condition in lowered
>  >for that would not be marked useful.
> Regardless, we already have code which will make that construct go away. So
> I'll ask again, why do we care if DCE marks these things as useful.  We're
> going to zap the entire bloody thing soon enough.

this is just what I wanted to hear.

>  >> However, what I have failed to consider is that if we keep the COND_EXPR
>  >> straightening code currently in tree_cleanup_cfg, and install your patch
>  >> to flatten COND_EXPRs, then we may be able remove all the control dependenc
>  >y
>  >> stuff in code in dce.c.
>  >> 
>  >> When I first attempted this on the assumption that if DCE killed all the
>  >> statements in both arms of a COND_EPXR that we should easily be able to
>  >> determine the COND_EXPR itself was uselss during cleanup_tree_cfg the
>  >> typical cases we missed looked something like
>  >> 
>  >> 
>  >> if (a)
>  >>   {
>  >>     {
>  >>       {
>  >>          (void)0
>  >>       }
>  >>     }
>  >>   }
>  >> else
>  >>   {
>  >>     {
>  >>       {
>  >>         (void)0
>  >>       }
>  >>     }
>  >>   }
>  >> 
>  >> [ Possibly with multiple empty statements strung throughout the arms. ]
>  >> 
>  >> Ie, DCE deleted all the executable statements, but since I had turned off
>  >> control statement removal, DCE left in the skeleton with the COND_EXPR.  
>  >> And since the arms were trivial empty statements, linearize_cond_expr could
>  >n't
>  >> do anything with it.
>  >> 
>  >> 
>  >> Now, if we keep linearize_cond_expr with your COND_EXPR lowering code we
>  >> would see 
>  >> 
>  >> if (a)
>  >>   {
>  >>     (void)0
>  >>   }
>  >> else
>  >>   {
>  >>     (void)0
>  >>   }
>  >
>  >uh?  Then you probably have other idea about the shape "lowered
>  >COND_EXPRs" should have.  In what I do, this would look like
>  >
>  >if (a)
>  >  goto x;
>  >else
>  >  goto y;
>  >
>  >x:
>  >  (void)0
>  >goto z;
>  >
>  >y:
>  > (void)0
>  >z:
>  >
>  >> Which linearize_cond_expr knows how to remove (and for which lazily build
>  >> pdoms -- so the vast majority of the time we don't have to pay the cost of
>  >> building pdoms.
>  >>
>  >> 
>  >> So what we _may_ want is your COND_EXPR lowering code, removal of the
>  >> control structure stuff in tree-ssa-dce.c and keep linearize_cond_expr.
>  >
>  >well, in fact I don't like linearize_cond_expr very much.  Instead we
>  >want jump threading (cfg cleanup version, i.e. over cfg cleanup only) +
>  >a simplified version of linearize_cond_expr that will just cancel
>  >conditions both of whose branches has the same target.
> Whatever.  The point is that by lowering COND_EXPRs, we no longer have
> to deal with control structures appearing as children of the COND_EXPR,
> which makes detection and elimination of if (x) goto y; else goto y
> trivial.  I don't care if we use linearlize_cond_expr or your jump
> threading code.
> 
> I think we're violently agreeing.

Seems so :-)

> Though given that I want to see us incrementally improve things, I do not
> want to see a mega patch which does both changes at once.

I would also prefer if the code in tree-ssa-dce was reworked before the
COND_EXPR patch.

> In fact, I'd really like to slow the patches down and get resolution
> on those which are already outstanding, starting with the LOOP_EXPR
> patch.  I believe there is a single outstanding issue with it that
> is preventing it from being installed (the gcov failures).

just sending a patch for this.

Zdenek


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