[PATCH] fix for bug 18319

[Nick Lewycky]

On 12/30/2013 07:48 AM, Ilia Filippov wrote:
> Here is an updated patch. I changed one test and added another one.

I made cosmetic adjustments (tests should use FileCheck instead of grep, 
added the PR# to the test, moved the 'else' onto the same line as the 
preceding '{') and committed it in r198267. Thanks for the patch!

Nick

>
>
> 2013/12/27 Nick Lewycky <nicholas at mxc.ca <mailto: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> <mailto: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>
>
>                  <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>
>         <mailto: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>
>              <http://lists.cs.uiuc.edu/__mailman/listinfo/llvm-commits
>         <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>>
>
>
>
>