[llvm] r243500 - ignore duplicate divisor uses when transforming into reciprocal multiplies (PR24141)

[Owen Anderson]

OK to merge.

—Owen

> On Jul 28, 2015, at 6:00 PM, Hans Wennborg <hans at chromium.org> wrote:
> 
> We should probably merge this to 3.7.
> 
> Owen, you're the owner here. OK to merge?
> 
> On Tue, Jul 28, 2015 at 4:28 PM, Sanjay Patel <spatel at rotateright.com> wrote:
>> Author: spatel
>> Date: Tue Jul 28 18:28:22 2015
>> New Revision: 243500
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=243500&view=rev
>> Log:
>> ignore duplicate divisor uses when transforming into reciprocal multiplies (PR24141)
>> 
>> PR24141: https://llvm.org/bugs/show_bug.cgi?id=24141
>> contains a test case where we have duplicate entries in a node's uses() list.
>> 
>> After r241826, we use CombineTo() to delete dead nodes when combining the uses into
>> reciprocal multiplies, but this fails if we encounter the just-deleted node again in
>> the list.
>> 
>> The solution in this patch is to not add duplicate entries to the list of users that
>> we will subsequently iterate over. For the test case, this avoids triggering the
>> combine divisors logic entirely because there really is only one user of the divisor.
>> 
>> Differential Revision: http://reviews.llvm.org/D11345
>> 
>> 
>> Modified:
>>    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>    llvm/trunk/test/CodeGen/X86/fdiv-combine.ll
>> 
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=243500&r1=243499&r2=243500&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Jul 28 18:28:22 2015
>> @@ -8261,11 +8261,11 @@ SDValue DAGCombiner::combineRepeatedFPDi
>>     return SDValue();
>> 
>>   // Find all FDIV users of the same divisor.
>> -  SmallVector<SDNode *, 4> Users;
>> -  for (auto *U : N1->uses()) {
>> +  // Use a set because duplicates may be present in the user list.
>> +  SetVector<SDNode *> Users;
>> +  for (auto *U : N1->uses())
>>     if (U->getOpcode() == ISD::FDIV && U->getOperand(1) == N1)
>> -      Users.push_back(U);
>> -  }
>> +      Users.insert(U);
>> 
>>   // Now that we have the actual number of divisor uses, make sure it meets
>>   // the minimum threshold specified by the target.
>> 
>> Modified: llvm/trunk/test/CodeGen/X86/fdiv-combine.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fdiv-combine.ll?rev=243500&r1=243499&r2=243500&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/fdiv-combine.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/fdiv-combine.ll Tue Jul 28 18:28:22 2015
>> @@ -44,5 +44,24 @@ define double @div3_arcp(double %x, doub
>>   ret double %ret
>> }
>> 
>> +define void @PR24141() #0 {
>> +; CHECK-LABEL: PR24141:
>> +; CHECK:       callq
>> +; CHECK-NEXT:  divsd
>> +; CHECK-NEXT:  jmp
>> +entry:
>> +  br label %while.body
>> +
>> +while.body:
>> +  %x.0 = phi double [ undef, %entry ], [ %div, %while.body ]
>> +  %call = call { double, double } @g(double %x.0)
>> +  %xv0 = extractvalue { double, double } %call, 0
>> +  %xv1 = extractvalue { double, double } %call, 1
>> +  %div = fdiv double %xv0, %xv1
>> +  br label %while.body
>> +}
>> +
>> +declare { double, double } @g(double)
>> +
>> ; FIXME: If the backend understands 'arcp', then this attribute is unnecessary.
>> attributes #0 = { "unsafe-fp-math"="true" }
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits