<div dir="ltr">On Thu, Jun 13, 2013 at 1:05 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Derek Schuff wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5">
Author: dschuff<br>
Date: Thu Jun 13 14:51:17 2013<br>
New Revision: 183933<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=183933&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=183933&view=rev</a><br>
Log:<br>
Fix DeleteDeadVarargs not to crash on functions referenced by BlockAddresses<br>
<br>
This pass was assuming that if hasAddressTaken() returns false for a<br>
function, the function's only uses are call sites.  That's not true<br>
because there can be references by BlockAddresses too.<br>
<br>
Fix the pass to handle this case.  Fix<br>
BlockAddress::<u></u>replaceUsesOfWithOnConstant() to allow a function's type<br>
to be changed by RAUW'ing the function with a bitcast of the recreated<br>
function.<br>
<br>
Patch by Mark Seaborn.<br>
<br>
Added:<br>
     llvm/trunk/test/Transforms/<u></u>DeadArgElim/2013-05-17-<u></u>VarargsAndBlockAddress.ll<br>
Modified:<br>
     llvm/trunk/lib/IR/Constants.<u></u>cpp<br>
     llvm/trunk/lib/Transforms/IPO/<u></u>DeadArgumentElimination.cpp<br>
<br>
Modified: llvm/trunk/lib/IR/Constants.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Constants.cpp?rev=183933&r1=183932&r2=183933&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/IR/<u></u>Constants.cpp?rev=183933&r1=<u></u>183932&r2=183933&view=diff</a><br>

==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/IR/Constants.<u></u>cpp (original)<br>
+++ llvm/trunk/lib/IR/Constants.<u></u>cpp Thu Jun 13 14:51:17 2013<br>
@@ -1389,7 +1389,7 @@ void BlockAddress::<u></u>replaceUsesOfWithOnCo<br>
    BasicBlock *NewBB = getBasicBlock();<br>
<br></div></div>
    if (U ==&Op<0>())<div class="im"><br>
-    NewF = cast<Function>(To);<br>
+    NewF = cast<Function>(To-><u></u>stripPointerCasts());<br>
</div></blockquote>
<br>
This makes me extremely nervous. This function is very low level, it seems like it shouldn't be doing anything like calling stripPointerCasts(), that'd be like rigging SmallVector to call eraseFromParent for you or something.<br>

<br>
What are the alternatives here, making BlockAddress hold a ConstantExpr instead of a Function? Because I can see why that's also very bad.<br></blockquote><div><br></div><div>We could also add a special ReplaceAllUsesWith variant specifically for this case, which doesn't require the types of the From and To values to be the same, and asserts all the uses are BlockAddresses.  I mean, the added call to <span style="font-family:arial,sans-serif;font-size:13px">replaceAllUsesWith is doing precisely what it's supposed to do; the part that's iffy is messing with the general case of ReplaceAllUsesWith.</span></div>
<div><div><br></div><div>-Eli</div></div></div></div></div>