[llvm] r286664 - [InstCombine] clean up foldSelectOpOp(); NFC

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 11 15:31:06 PST 2016


I knew that still didn't look right. Should be fixed with r286671 - thanks!

On Fri, Nov 11, 2016 at 4:20 PM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 11/11/2016 3:01 PM, Sanjay Patel via llvm-commits wrote:
>
>> Author: spatel
>> Date: Fri Nov 11 17:01:20 2016
>> New Revision: 286664
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=286664&view=rev
>> Log:
>> [InstCombine] clean up foldSelectOpOp(); NFC
>>
>> Modified:
>>      llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>> s/InstCombine/InstCombineSelect.cpp?rev=286664&r1=286663&r2=
>> 286664&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Fri Nov
>> 11 17:01:20 2016
>> @@ -162,8 +162,6 @@ Instruction *InstCombiner::foldSelectOpO
>>                               TI->getType());
>>     }
>>   -  // TODO: This function ends awkwardly in unreachable - fix to be
>> more normal.
>> -
>>     // Only handle binary operators with one-use here. As with the cast
>> case
>>     // above, it may be possible to relax the one-use constraint, but
>> that needs
>>     // be examined carefully since it may not reduce the total number of
>> @@ -203,14 +201,10 @@ Instruction *InstCombiner::foldSelectOpO
>>     // If we reach here, they do have operations in common.
>>     Value *NewSI = Builder->CreateSelect(SI.getCondition(), OtherOpT,
>> OtherOpF,
>>                                          SI.getName() + ".v", &SI);
>> -
>> -  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(TI)) {
>> -    if (MatchIsOpZero)
>> -      return BinaryOperator::Create(BO->getOpcode(), MatchOp, NewSI);
>> -    else
>> -      return BinaryOperator::Create(BO->getOpcode(), NewSI, MatchOp);
>> -  }
>> -  llvm_unreachable("Shouldn't get here");
>> +  BinaryOperator *BO = cast<BinaryOperator>(TI);
>>
>
> It would be better to use dyn_cast<> rather than the isa<>+cast<>
> anti-pattern.
>
> -Eli
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161111/96a1f7c5/attachment.html>


More information about the llvm-commits mailing list