[PATCH] fix for bug 18319

[Ilia Filippov]

Here is an updated patch. I changed one test and added another one.


2013/12/27 Nick Lewycky <nicholas at mxc.ca>

> On 12/27/2013 06:45 AM, Ilia Filippov wrote:
>
>> Here is an updated patch, please review
>>
>
> The new logic looks great, thanks!
>
> These two lines are over 80 columns:
>
> +      Constant *V1Element = ConstantExpr::getExtractElement(V1,
> ConstantInt::get(Ty, i));
> +      Constant *V2Element = ConstantExpr::getExtractElement(V2,
> ConstantInt::get(Ty, i));
>
> +      if (V1Element == V2Element)
> +        V = V1Element;
> +      else if (isa<UndefValue>(Cond))
> +        V = isa<UndefValue>(V1Element) ? V1Element : V2Element;
> +      else {
>
> 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.
>
> And you need to include a testcase.
>
> Nick
>
>  2013/12/25 Nick Lewycky <nicholas at mxc.ca <mailto:nicholas at mxc.ca>>
>>
>>
>>     On 12/24/2013 07:18 AM, Ilia Filippov wrote:
>>
>>         Hello,
>>
>>         Could somebody please review and commit my fix to bug
>>         http://llvm.org/bugs/show_bug.__cgi?id=18319
>>
>>         <http://llvm.org/bugs/show_bug.cgi?id=18319> ?
>>         (patch is attached).
>>
>>         The fix handles the case of vector select constant expression
>>         containing
>>         "undef" values in ConstantFoldSelectInstruction function.
>>
>>
>>     Index: lib/IR/ConstantFold.cpp
>>     ==============================__============================
>> ==__=======
>>
>>     --- lib/IR/ConstantFold.cpp     (revision 197903)
>>     +++ lib/IR/ConstantFold.cpp     (working copy)
>>     @@ -706,9 +706,15 @@
>>           Type *Ty = IntegerType::get(CondV->__getContext(), 32);
>>
>>           for (unsigned i = 0, e =
>>     V1->getType()->__getVectorNumElements(); i != e;++i){
>>             ConstantInt *Cond =
>>     dyn_cast<ConstantInt>(CondV->__getOperand(i));
>>
>>     -      if (Cond == 0) break;
>>     -
>>     -      Constant *V = Cond->isNullValue() ? V2 : V1;
>>     +      Constant *V;
>>     +      if (Cond == 0) {
>>     +        if (isa<UndefValue>(CondV->__getOperand(i)))
>>
>>
>>     Instead of re-getting from CondV, make Cond a Constant* instead of a
>>     ConstantInt*, and have it do "if (!isa<ConstantInt>(Cond)) break;"
>>     after handling the undef case.
>>
>>     There's some other logic missing here though. If
>>     cast<ConstantVector>(V1)->__getOperand(n) is equal to
>>     cast<ConstantVector>(V2)->__getOperand(n), then we don't care what
>>
>>     Cond is at all, we should just pick that element.
>>
>>     Since we're going to look at each element this way, why not check if
>>     one of the elements is better than the other (ie., is an undef) and
>>     then pick that element when the condition is undef?
>>
>>     Nick
>>
>>     +          V = V2;
>>     +        else
>>     +          break;
>>     +      }
>>     +      else
>>     +        V = Cond->isNullValue() ? V2 : V1;
>>             Constant *Res = ConstantExpr::__getExtractElement(V,
>>     ConstantInt::get(Ty, i));
>>             Result.push_back(Res);
>>           }
>>
>>     _________________________________________________
>>     llvm-commits mailing list
>>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/__mailman/listinfo/llvm-commits
>>     <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131230/eaf8cbde/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.patch
Type: application/octet-stream
Size: 2422 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131230/eaf8cbde/attachment.obj>