<div class="gmail_quote">On 27 January 2011 10:11, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="im">On Jan 27, 2011, at 12:38 AM, Nick Lewycky wrote:<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=124368&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=124368&view=rev</a><br>
> Log:<br>
> Fix surprising missed optimization in mergefunc where we forgot to consider<br>
> that relationships like "i8* null" is equivalent to "i32* null".<br>
<br>
</div>Nice!<br>
<div class="im"><br>
> +++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Thu Jan 27 02:38:19 2011<br>
> @@ -460,9 +460,18 @@<br>
<br>
</div><div class="im">> +  if (isa<Constant>(V1)) {<br>
> +    if (V1 == V2) return true;<br>
> +    const Constant *C1 = cast<Constant>(V1);<br>
<br>
</div>Please use dyn_cast in the if instead of isa+cast.<br></blockquote><div><br></div><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">> +    const Constant *C2 = dyn_cast<Constant>(V2);<br>
> +    if (!C2) return false;<br>
> +    // TODO: constant expressions with GEP or references to F1 or F2.<br>
> +    if (C1->isNullValue() && C2->isNullValue() &&<br>
> +     isEquivalentType(C1->getType(), C2->getType()))<br>
> +      return true;<br>
<br>
</div>isEquivalentType accepts intptr_t and void* as equiv, so this will merge null with (intptr)0 right?<br></blockquote><div><br></div><div>Right.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">


<div class="im">> +    return C1->getType()->canLosslesslyBitCastTo(C2->getType()) &&<br>
> +      C1 == ConstantExpr::getBitCast(const_cast<Constant*>(C2), C1->getType());<br>
> +  }<br>
<br>
</div>Urr?  What is this about?  This needs a comment at the very least.<br></blockquote><div><br></div><div>I've added a comment. The only case this doesn't handle is inttoptr/ptrtoint conversions (since you can't bitcast integers and pointers), which are special-cased for null-only above. I may want to generalize that test too.</div>

<div><br></div><div>While we're at it, I should probably also call ConstantFoldConstantExpression on it given that I have a TD and ConstantExpr doesn't. I didn't think it was worth thinking about this comparison function too hard, but maybe I'll try it and see whether it improves the number of merges in llvm-test.</div>

<div><br></div><div>Nick</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<font color="#888888"><br>
-Chris<br>
</font><div><div></div><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br>