On 05/15/2018 04:47 PM, Kugan Vivekanandarajah wrote: > Hi Richard, > > On 15 May 2018 at 19:20, Richard Biener <rguent...@suse.de> wrote: >> On Tue, 15 May 2018, Richard Biener wrote: >> >>> On Mon, 14 May 2018, Kugan Vivekanandarajah wrote: >>> >>>> Hi, >>>> >>>> Attached patch handles PR63185 when we reach PHI with temp != NULLL. >>>> We could see the PHI and if there isn't any uses for PHI that is >>>> interesting, we could ignore that ? >>>> >>>> Bootstrapped and regression tested on x86_64-linux-gnu. >>>> Is this OK? >>> >>> No, as Jeff said we can't do it this way. >>> >>> If we end up with multiple VDEFs in the walk of defvar immediate >>> uses we know we are dealing with a CFG fork. We can't really >>> ignore any of the paths but we have to >>> >>> a) find the merge point (and the associated VDEF) >>> b) verify for each each chain of VDEFs with associated VUSEs >>> up to that merge VDEF that we have no uses of the to classify >>> store and collect (partial) kills >>> c) intersect kill info and continue walking from the merge point >>> >>> in b) there's the optional possibility to find sinking opportunities >>> in case we have kills on some paths but uses on others. This is why >>> DSE should be really merged with (store) sinking. >>> >>> So if we want to enhance DSEs handling of branches then we need >>> to refactor the simple dse_classify_store function. Let me take >>> an attempt at this today. >> >> 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). > > Thanks for the explanation and thanks for working on this. > >> >> I took the opportunity to kill the use_stmt parameter of >> dse_classify_store as the only user is only looking for whether >> the kills were all clobbers which I added a new parameter for. >> >> 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). > > Since byte tracking is tracking the killed subset of bytes in the > original def, in your patch can we calculate the bytes killed by each > defs[i] and then find the intersection for the iteration. If we don't > have complete kill, we can use this to set live bytes and iterate till > all the live_bytes are cleared like it was done earlier. Correct.
Jeff
