<div dir="ltr">Here is an updated patch. I changed one test and added another one.</div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/12/27 Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 12/27/2013 06:45 AM, Ilia Filippov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Here is an updated patch, please review<br>
</blockquote>
<br></div>
The new logic looks great, thanks!<br>
<br>
These two lines are over 80 columns:<br>
<br>
+ Constant *V1Element = ConstantExpr::<u></u>getExtractElement(V1, ConstantInt::get(Ty, i));<br>
+ Constant *V2Element = ConstantExpr::<u></u>getExtractElement(V2, ConstantInt::get(Ty, i));<br>
<br>
+ if (V1Element == V2Element)<br>
+ V = V1Element;<br>
+ else if (isa<UndefValue>(Cond))<br>
+ V = isa<UndefValue>(V1Element) ? V1Element : V2Element;<br>
+ else {<br>
<br>
Please add braces around each stanza, even though they're only 1 line. The idea is that one of the sides (the final else in this case) has braces, so too should the other stanzas.<br>
<br>
And you need to include a testcase.<br>
<br>
Nick<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2013/12/25 Nick Lewycky <<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a> <mailto:<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>>><div class="im"><br>
<br>
On 12/24/2013 07:18 AM, Ilia Filippov wrote:<br>
<br>
Hello,<br>
<br>
Could somebody please review and commit my fix to bug<br></div>
<a href="http://llvm.org/bugs/show_bug.__cgi?id=18319" target="_blank">http://llvm.org/bugs/show_bug.<u></u>__cgi?id=18319</a><div class="im"><br>
<<a href="http://llvm.org/bugs/show_bug.cgi?id=18319" target="_blank">http://llvm.org/bugs/show_<u></u>bug.cgi?id=18319</a>> ?<br>
(patch is attached).<br>
<br>
The fix handles the case of vector select constant expression<br>
containing<br>
"undef" values in ConstantFoldSelectInstruction function.<br>
<br>
<br>
Index: lib/IR/ConstantFold.cpp<br></div>
==============================<u></u>__============================<u></u>==__=======<div class="im"><br>
--- lib/IR/ConstantFold.cpp (revision 197903)<br>
+++ lib/IR/ConstantFold.cpp (working copy)<br>
@@ -706,9 +706,15 @@<br></div>
Type *Ty = IntegerType::get(CondV->__<u></u>getContext(), 32);<div class="im"><br>
for (unsigned i = 0, e =<br></div>
V1->getType()->__<u></u>getVectorNumElements(); i != e;++i){<br>
ConstantInt *Cond =<br>
dyn_cast<ConstantInt>(CondV->_<u></u>_getOperand(i));<div class="im"><br>
- if (Cond == 0) break;<br>
-<br>
- Constant *V = Cond->isNullValue() ? V2 : V1;<br>
+ Constant *V;<br>
+ if (Cond == 0) {<br></div>
+ if (isa<UndefValue>(CondV->__<u></u>getOperand(i)))<div class="im"><br>
<br>
Instead of re-getting from CondV, make Cond a Constant* instead of a<br>
ConstantInt*, and have it do "if (!isa<ConstantInt>(Cond)) break;"<br>
after handling the undef case.<br>
<br>
There's some other logic missing here though. If<br></div>
cast<ConstantVector>(V1)->__<u></u>getOperand(n) is equal to<br>
cast<ConstantVector>(V2)->__<u></u>getOperand(n), then we don't care what<div class="im"><br>
Cond is at all, we should just pick that element.<br>
<br>
Since we're going to look at each element this way, why not check if<br>
one of the elements is better than the other (ie., is an undef) and<br>
then pick that element when the condition is undef?<br>
<br>
Nick<br>
<br>
+ V = V2;<br>
+ else<br>
+ break;<br>
+ }<br>
+ else<br>
+ V = Cond->isNullValue() ? V2 : V1;<br></div>
Constant *Res = ConstantExpr::__<u></u>getExtractElement(V,<br>
ConstantInt::get(Ty, i));<br>
Result.push_back(Res);<br>
}<br>
<br>
______________________________<u></u>___________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/__mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/__<u></u>mailman/listinfo/llvm-commits</a><br>
<<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a>><br>
<br>
<br>
</blockquote>
<br>
</blockquote></div><br></div>