<div dir="ltr">On 18 August 2013 17:06, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 08/15/2013 10:06 AM, Paul Berry wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 12 August 2013 13:11, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a><br></div><div class="im">
<mailto:<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><u></u>> wrote:<br>
<br>
    fs_copy_prop_dataflow::setup_<u></u>kills() walks through each basic block's<br>
    instructions, looking for instructions which overwrite registers used<br>
    in ACP entries.<br>
<br>
    This would be fine, except that it didn't exclude the instructions<br>
    which generated the ACP entries in the first place.  This meant that<br>
    every copy was killed by its own generating instruction, making the<br>
    dataflow analysis useless.<br>
<br>
<br>
I don't think this is actually a problem, because the bd[b].kills bits<br>
are only used to prevent liveness information from being propagated from<br>
bd[b].livein to bd[b].liveout, and the fs_copy_prop_dataflow constructor<br>
initializes bd[b].liveout with the set of ACP's that are generated from<br>
block b.<br>
</div></blockquote>
<br>
It doesn't by the end of the series, but I think that's moot...</blockquote><div><br></div><div>Fair point.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
So the only situation in which your patch would allow liveness<br>
information to be propagated from bd[b].livein to bd[b].liveout is for<br>
bits in bd[b].liveout that are already set.<br>
<br>
Or am I misunderstanding things?<br>
</blockquote>
<br></div>
I think you're right that this is unnecessary.  The user of kill is:<br>
<br>
   bd[b].liveout[i] =<br>
      bd[b].copy[i] | (bd[b].livein[i] & ~bd[b].kill[i]);<br>
<br>
So even though copies are marked as killed, they'll be reintroduced as liveout due to the union with the COPY set.  With the rest of the bugs in the algorithm fixed, dropping this patch makes no difference.<br>
<br>
I'm happy to drop it.  Would you prefer that?<br>
</blockquote></div><br></div><div class="gmail_extra">Yeah, I think I have a minor preference for dropping it.  But I won't press the issue.<br></div></div>