[llvm] r278944 - [InstCombine] clean up foldICmpOrConstant(); NFCI

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 10:05:12 PDT 2016


Fixed in  r279271


On Fri, Aug 19, 2016 at 9:45 AM, Sanjay Patel <spatel at rotateright.com>
wrote:

> Oops! Sorry about that. Thank you for letting me know.
>
> On Fri, Aug 19, 2016 at 10:37 AM, Reid Kleckner <rnk at google.com> wrote:
>
>> Er, the actual logs this time:
>> https://build.chromium.org/p/chromium.fyi/builders/ClangToTL
>> inux/builds/6077/steps/compile/logs/stdio
>>
>> I'll land a quick fix and test soon.
>>
>> On Fri, Aug 19, 2016 at 9:36 AM, Reid Kleckner <rnk at google.com> wrote:
>>
>>> +    Constant *NullVal = ConstantInt::getNullValue(P->getType());
>>> +    Value *CmpP = Builder->CreateICmp(Pred, P, NullVal);
>>> +    Value *CmpQ = Builder->CreateICmp(Pred, Q, NullVal);
>>>
>>> P and Q might not have equivalent pointer types. We really do need to
>>> get two calls to getNullValue here.
>>>
>>> Caused a crash while building Chromium:
>>>
>>> On Wed, Aug 17, 2016 at 9:30 AM, Sanjay Patel via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: spatel
>>>> Date: Wed Aug 17 11:30:43 2016
>>>> New Revision: 278944
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=278944&view=rev
>>>> Log:
>>>> [InstCombine] clean up foldICmpOrConstant(); NFCI
>>>>
>>>> 1. Change variable names
>>>> 2. Use local variables to reduce code
>>>> 3. Use ? instead of if/else
>>>> 4. Use the APInt variable instead of 'RHS' so the removal of the FIXME
>>>> code will be direct
>>>>
>>>> Modified:
>>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>> s/InstCombine/InstCombineCompares.cpp?rev=278944&r1=278943&r
>>>> 2=278944&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>>>> (original)
>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Wed
>>>> Aug 17 11:30:43 2016
>>>> @@ -1879,40 +1879,38 @@ Instruction *InstCombiner::foldICmpAndCo
>>>>    return nullptr;
>>>>  }
>>>>
>>>> -Instruction *InstCombiner::foldICmpOrConstant(ICmpInst &ICI,
>>>> Instruction *LHSI,
>>>> -                                              const APInt *RHSV) {
>>>> +/// Fold icmp (or X, Y), C.
>>>> +Instruction *InstCombiner::foldICmpOrConstant(ICmpInst &Cmp,
>>>> Instruction *Or,
>>>> +                                              const APInt *C) {
>>>>    // FIXME: This check restricts all folds under here to scalar types.
>>>> -  ConstantInt *RHS = dyn_cast<ConstantInt>(ICI.getOperand(1));
>>>> +  ConstantInt *RHS = dyn_cast<ConstantInt>(Cmp.getOperand(1));
>>>>    if (!RHS)
>>>>      return nullptr;
>>>>
>>>> -  if (RHS->isOne()) {
>>>> +  ICmpInst::Predicate Pred = Cmp.getPredicate();
>>>> +  if (*C == 1) {
>>>>      // icmp slt signum(V) 1 --> icmp slt V, 1
>>>>      Value *V = nullptr;
>>>> -    if (ICI.getPredicate() == ICmpInst::ICMP_SLT &&
>>>> -        match(LHSI, m_Signum(m_Value(V))))
>>>> +    if (Pred == ICmpInst::ICMP_SLT && match(Or, m_Signum(m_Value(V))))
>>>>        return new ICmpInst(ICmpInst::ICMP_SLT, V,
>>>>                            ConstantInt::get(V->getType(), 1));
>>>>    }
>>>>
>>>> -  if (!ICI.isEquality() || !RHS->isNullValue() || !LHSI->hasOneUse())
>>>> +  if (!Cmp.isEquality() || *C != 0 || !Or->hasOneUse())
>>>>      return nullptr;
>>>>
>>>>    Value *P, *Q;
>>>> -  if (match(LHSI, m_Or(m_PtrToInt(m_Value(P)),
>>>> m_PtrToInt(m_Value(Q))))) {
>>>> +  if (match(Or, m_Or(m_PtrToInt(m_Value(P)), m_PtrToInt(m_Value(Q)))))
>>>> {
>>>>      // Simplify icmp eq (or (ptrtoint P), (ptrtoint Q)), 0
>>>>      // -> and (icmp eq P, null), (icmp eq Q, null).
>>>> -    Value *ICIP = Builder->CreateICmp(ICI.getPredicate(), P,
>>>> -                                      Constant::getNullValue(P->getT
>>>> ype()));
>>>> -    Value *ICIQ = Builder->CreateICmp(ICI.getPredicate(), Q,
>>>> -                                      Constant::getNullValue(Q->getT
>>>> ype()));
>>>> -    Instruction *Op;
>>>> -    if (ICI.getPredicate() == ICmpInst::ICMP_EQ)
>>>> -      Op = BinaryOperator::CreateAnd(ICIP, ICIQ);
>>>> -    else
>>>> -      Op = BinaryOperator::CreateOr(ICIP, ICIQ);
>>>> -    return Op;
>>>> +    Constant *NullVal = ConstantInt::getNullValue(P->getType());
>>>> +    Value *CmpP = Builder->CreateICmp(Pred, P, NullVal);
>>>> +    Value *CmpQ = Builder->CreateICmp(Pred, Q, NullVal);
>>>> +    auto LogicOpc = Pred == ICmpInst::Predicate::ICMP_EQ ?
>>>> Instruction::And
>>>> +                                                         :
>>>> Instruction::Or;
>>>> +    return BinaryOperator::Create(LogicOpc, CmpP, CmpQ);
>>>>    }
>>>> +
>>>>    return nullptr;
>>>>  }
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160819/ca56792a/attachment.html>


More information about the llvm-commits mailing list