[llvm] r247061 - refactor matches for De Morgan's Laws; NFCI

[Sanjay Patel via llvm-commits]

No problem. Thanks for reviewing!


On Wed, Sep 9, 2015 at 3:03 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

>
>
> On Wed, Sep 9, 2015 at 2:00 PM, Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>> Sorry, I'm not understanding. Are you referring to the opcode selection
>> of 'and' / 'or' before calling CreateBinOp()? That's a result of hoisting
>> the code so it can be called from visitAnd / visitOr, and I added the
>> assert to make sure this helper wouldn't be misused. Let me know if there's
>> a hole in that logic.
>>
>>
> This change is fine, sorry about that.  My mail client didn't render the
> commit correctly :/
>
>
>>
>> On Wed, Sep 9, 2015 at 2:27 PM, David Majnemer <david.majnemer at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Sep 8, 2015 at 1:14 PM, Sanjay Patel via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: spatel
>>>> Date: Tue Sep  8 15:14:13 2015
>>>> New Revision: 247061
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=247061&view=rev
>>>> Log:
>>>> refactor matches for De Morgan's Laws; NFCI
>>>>
>>>> Modified:
>>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=247061&r1=247060&r2=247061&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
>>>> (original)
>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Tue
>>>> Sep  8 15:14:13 2015
>>>> @@ -1200,6 +1200,34 @@ Value *InstCombiner::FoldAndOfFCmps(FCmp
>>>>    return nullptr;
>>>>  }
>>>>
>>>> +/// Match De Morgan's Laws:
>>>> +/// (~A & ~B) == (~(A | B))
>>>> +/// (~A | ~B) == (~(A & B))
>>>> +static Instruction *matchDeMorgansLaws(BinaryOperator &I,
>>>> +                                       InstCombiner::BuilderTy
>>>> *Builder) {
>>>> +  auto Opcode = I.getOpcode();
>>>> +  assert((Opcode == Instruction::And || Opcode == Instruction::Or) &&
>>>> +         "Trying to match De Morgan's Laws with something other than
>>>> and/or");
>>>> +
>>>> +  Value *Op0 = I.getOperand(0);
>>>> +  Value *Op1 = I.getOperand(1);
>>>> +  // TODO: Use pattern matchers instead of dyn_cast.
>>>> +  if (Value *Op0NotVal = dyn_castNotVal(Op0))
>>>> +    if (Value *Op1NotVal = dyn_castNotVal(Op1))
>>>> +      if (Op0->hasOneUse() && Op1->hasOneUse()) {
>>>> +        // Flip the logic operation.
>>>> +        if (Opcode == Instruction::And)
>>>> +          Opcode = Instruction::Or;
>>>> +        else
>>>> +          Opcode = Instruction::And;
>>>>
>>>
>>> Excuse me but how is this "no functional change" ?  I don't believe we
>>> had the logic to conditional create an Or before.
>>>
>>>
>>>> +        Value *LogicOp = Builder->CreateBinOp(Opcode, Op0NotVal,
>>>> Op1NotVal,
>>>> +                                              I.getName() +
>>>> ".demorgan");
>>>> +        return BinaryOperator::CreateNot(LogicOp);
>>>> +      }
>>>> +
>>>> +  return nullptr;
>>>> +}
>>>> +
>>>>  Instruction *InstCombiner::visitAnd(BinaryOperator &I) {
>>>>    bool Changed = SimplifyAssociativeOrCommutative(I);
>>>>    Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
>>>> @@ -1330,15 +1358,8 @@ Instruction *InstCombiner::visitAnd(Bina
>>>>          return NV;
>>>>    }
>>>>
>>>> -
>>>> -  // (~A & ~B) == (~(A | B)) - De Morgan's Law
>>>> -  if (Value *Op0NotVal = dyn_castNotVal(Op0))
>>>> -    if (Value *Op1NotVal = dyn_castNotVal(Op1))
>>>> -      if (Op0->hasOneUse() && Op1->hasOneUse()) {
>>>> -        Value *Or = Builder->CreateOr(Op0NotVal, Op1NotVal,
>>>> -                                      I.getName()+".demorgan");
>>>> -        return BinaryOperator::CreateNot(Or);
>>>> -      }
>>>> +  if (Instruction *DeMorgan = matchDeMorgansLaws(I, Builder))
>>>> +    return DeMorgan;
>>>>
>>>>    {
>>>>      Value *A = nullptr, *B = nullptr, *C = nullptr, *D = nullptr;
>>>> @@ -2360,14 +2381,8 @@ Instruction *InstCombiner::visitOr(Binar
>>>>    if (match(Op0, m_And(m_Or(m_Specific(Op1), m_Value(C)), m_Value(A))))
>>>>      return BinaryOperator::CreateOr(Op1, Builder->CreateAnd(A, C));
>>>>
>>>> -  // (~A | ~B) == (~(A & B)) - De Morgan's Law
>>>> -  if (Value *Op0NotVal = dyn_castNotVal(Op0))
>>>> -    if (Value *Op1NotVal = dyn_castNotVal(Op1))
>>>> -      if (Op0->hasOneUse() && Op1->hasOneUse()) {
>>>> -        Value *And = Builder->CreateAnd(Op0NotVal, Op1NotVal,
>>>> -                                        I.getName()+".demorgan");
>>>> -        return BinaryOperator::CreateNot(And);
>>>> -      }
>>>> +  if (Instruction *DeMorgan = matchDeMorgansLaws(I, Builder))
>>>> +    return DeMorgan;
>>>>
>>>>    // Canonicalize xor to the RHS.
>>>>    bool SwappedForXor = false;
>>>>
>>>>
>>>> _______________________________________________
>>>> 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/20150909/5e6fcf96/attachment.html>