On 05/15/2018 03:20 AM, Richard Biener wrote: > > First (baby) step is the following - it arranges to collect the > defs we need to continue walking from and implements trivial > reduction by stopping at (full) kills. This allows us to handle > the new testcase (which was already handled in the very late DSE > pass with the help of sinking the store). Seems like a notable improvement. They way we were handling defs to continue walking forward from was, umm, fugly.
> > I didn't adjust the byte-tracking case fully (I'm not fully understanding > the code in the case of a use and I'm not sure whether it's worth > doing the def reduction with byte-tracking). The byte tracking case is pretty simple. We're tracking the bytes from the original store that are still live. If we encounter a subsequent store that over-writes a subset of bytes, we consider those bytes from the original store as "dead". If we then encounter a load and the load only reads from "dead" bytes, then we can ignore the load for the purposes of DSE. It's something you suggested. I did some instrumentation when working on DSE for gcc6/gcc7 and while it triggers, it's not terribly often. > > Your testcase can be handled by reducing the PHI and the call def > by seeing that the only use of a candidate def is another def > we have already processed. Not sure if worth special-casing though, > I'd rather have a go at "recursing". That will be the next > patch. > > Bootstrap & regtest running on x86_64-unknown-linux-gnu. > > Richard. > > 2018-05-15 Richard Biener <rguent...@suse.de> > > * tree-ssa-dse.c (dse_classify_store): Remove use_stmt parameter, > add by_clobber_p one. Change algorithm to collect all defs > representing uses we need to walk and try reducing them to > a single one before failing. > (dse_dom_walker::dse_optimize_stmt): Adjust. > > * gcc.dg/tree-ssa/ssa-dse-31.c: New testcase. Seems quite reasonable to me as long as we're continuing with a forward algorithm. jeff
