<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 5, 2014 at 11:43 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">Björn Steinbrink wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In transformConstExprCastCall, if the replaced call has no users, we<br>
currently don't call ReplaceInstUsesWith, which means that ValueHandlers<br>
aren't notified about the replacement, but only about the removal of the<br>
old call.<br>
<br>
This means that we can't immediately detect the devirtualization of such<br>
calls when updating the call graph in the CGSCC pass, because the old<br>
CallSite got invalidated. If some other change then fools the simple<br>
counting logic, we fail to re-run the pass on the current function,<br>
missing further optimization possibilities opened up by the<br>
devirtualized call.<br>
<br>
As a fix, we should call ReplaceInstUsesWith even if there are no users.<br>
The only exception is when the return value changed, because we don't<br>
add a cast for it when there are no users, and ReplaceInstUsesWith would<br>
hit an assertion in that case.<br>
</blockquote>
<br></div>
The code to handle that case is right above it, remove the !Caller->use_empty() test on line 1208?<br>
<br>
   1208   if (OldRetTy != NV->getType() && !Caller->use_empty()) {<br>
<br>
Overall I'm torn on this one. I don't like the notion of having optimization passes do extra work just to signal things to value handle holders. The point of the VH is to not make us need to change the optz'ns to follow what's going on. A more explicit way to show what's going on would be to call getPassIfAvailable<CallGraph> and use its API to keep it up to date.<br>

<br>
On the other hand that would feel out of place here, and this is an important issue to get right. The ad-hoc test that the CGSCC PM does wrong anyways, and the notion of holding the CallGraph as "correct" while these function passes don't preserve it and modify the call graph is dubious at best.<br>

<br>
I'll accept this approach of doing an extra RAUW to notify the callgraph. I'd appreciate if you could make the RAUW itself unconditional.</blockquote></div><br>I share your feelings Nick. For my part (having studdied this recently in the process of working on a replacement for the new pass manager) I think the correct thing is to form a cast and *always* RAUW. As much as I dislike using RAUW as a magical signal to ValueHandles, I dislike inconsistently iterating on the function to discover further simplifications much, much more.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">-Chandler</div></div>