From: Sanjay Patel Date: Thu, 6 Dec 2018 18:16:32 +0000 (+0000) Subject: [DAGCombiner] don't hoist logic op if operands have other uses X-Git-Tag: llvmorg-8.0.0-rc1~2699 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c3717cd0d557ac2df0ce8a58487aacd8a502e69d;p=platform%2Fupstream%2Fllvm.git [DAGCombiner] don't hoist logic op if operands have other uses The AVX512 diffs are neutral, but the bswap test shows a clear overreach in hoistLogicOpWithSameOpcodeHands(). If we don't check for other uses, we can increase the instruction count. This could also fight with transforms trying to go in the opposite direction and possibly blow up/infinite loop. This might be enough to solve the bug noted here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20181203/608593.html I did not add the hasOneUse() checks to all opcodes because I see a perf regression for at least one opcode. We may decide that's irrelevant in the face of potential compiler crashing, but I'll see if I can salvage that first. llvm-svn: 348508 --- diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index d9290c6..5a70dee 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -3715,8 +3715,8 @@ SDValue DAGCombiner::hoistLogicOpWithSameOpcodeHands(SDNode *N) { if (N0.getNumOperands() == 0) return SDValue(); - // FIXME: We need to check number of uses of the operands to not increase - // the instruction count. + // FIXME: We should check number of uses of the operands to not increase + // the instruction count for all transforms. EVT Op0VT = N0.getOperand(0).getValueType(); switch (HandOpcode) { @@ -3725,6 +3725,10 @@ SDValue DAGCombiner::hoistLogicOpWithSameOpcodeHands(SDNode *N) { case ISD::ZERO_EXTEND: case ISD::SIGN_EXTEND: case ISD::BSWAP: + // If both operands have other uses, this transform would create extra + // instructions without eliminating anything. + if (!N0.hasOneUse() && !N1.hasOneUse()) + return SDValue(); // We need matching integer source types. // Do not hoist logic op inside of a vector extend, since it may combine // into a vsetcc. diff --git a/llvm/test/CodeGen/X86/avx512-mask-op.ll b/llvm/test/CodeGen/X86/avx512-mask-op.ll index 28ce5bb..5996de5 100644 --- a/llvm/test/CodeGen/X86/avx512-mask-op.ll +++ b/llvm/test/CodeGen/X86/avx512-mask-op.ll @@ -152,8 +152,8 @@ define i16 @mand16(i16 %x, i16 %y) { ; CHECK: ## %bb.0: ; CHECK-NEXT: movl %edi, %eax ; CHECK-NEXT: movl %edi, %ecx -; CHECK-NEXT: xorl %esi, %ecx -; CHECK-NEXT: andl %esi, %eax +; CHECK-NEXT: andl %esi, %ecx +; CHECK-NEXT: xorl %esi, %eax ; CHECK-NEXT: orl %ecx, %eax ; CHECK-NEXT: ## kill: def $ax killed $ax killed $eax ; CHECK-NEXT: retq diff --git a/llvm/test/CodeGen/X86/avx512-schedule.ll b/llvm/test/CodeGen/X86/avx512-schedule.ll index 3bfe088..5c44d86 100755 --- a/llvm/test/CodeGen/X86/avx512-schedule.ll +++ b/llvm/test/CodeGen/X86/avx512-schedule.ll @@ -6795,8 +6795,8 @@ define i16 @mand16(i16 %x, i16 %y) { ; GENERIC: # %bb.0: ; GENERIC-NEXT: movl %edi, %eax # sched: [1:0.33] ; GENERIC-NEXT: movl %edi, %ecx # sched: [1:0.33] -; GENERIC-NEXT: xorl %esi, %ecx # sched: [1:0.33] -; GENERIC-NEXT: andl %esi, %eax # sched: [1:0.33] +; GENERIC-NEXT: andl %esi, %ecx # sched: [1:0.33] +; GENERIC-NEXT: xorl %esi, %eax # sched: [1:0.33] ; GENERIC-NEXT: orl %ecx, %eax # sched: [1:0.33] ; GENERIC-NEXT: # kill: def $ax killed $ax killed $eax ; GENERIC-NEXT: retq # sched: [1:1.00] @@ -6805,8 +6805,8 @@ define i16 @mand16(i16 %x, i16 %y) { ; SKX: # %bb.0: ; SKX-NEXT: movl %edi, %eax # sched: [1:0.25] ; SKX-NEXT: movl %edi, %ecx # sched: [1:0.25] -; SKX-NEXT: xorl %esi, %ecx # sched: [1:0.25] -; SKX-NEXT: andl %esi, %eax # sched: [1:0.25] +; SKX-NEXT: andl %esi, %ecx # sched: [1:0.25] +; SKX-NEXT: xorl %esi, %eax # sched: [1:0.25] ; SKX-NEXT: orl %ecx, %eax # sched: [1:0.25] ; SKX-NEXT: # kill: def $ax killed $ax killed $eax ; SKX-NEXT: retq # sched: [7:1.00] diff --git a/llvm/test/CodeGen/X86/avx512dq-mask-op.ll b/llvm/test/CodeGen/X86/avx512dq-mask-op.ll index 06bbeef..3d6f53d 100644 --- a/llvm/test/CodeGen/X86/avx512dq-mask-op.ll +++ b/llvm/test/CodeGen/X86/avx512dq-mask-op.ll @@ -33,10 +33,10 @@ define i8 @mand8(i8 %x, i8 %y) { ; CHECK-LABEL: mand8: ; CHECK: ## %bb.0: ; CHECK-NEXT: movl %edi, %eax -; CHECK-NEXT: movl %edi, %ecx -; CHECK-NEXT: xorl %esi, %ecx -; CHECK-NEXT: andl %esi, %eax -; CHECK-NEXT: orl %ecx, %eax +; CHECK-NEXT: movl %eax, %ecx +; CHECK-NEXT: andb %sil, %cl +; CHECK-NEXT: xorb %sil, %al +; CHECK-NEXT: orb %cl, %al ; CHECK-NEXT: ## kill: def $al killed $al killed $eax ; CHECK-NEXT: retq %ma = bitcast i8 %x to <8 x i1> diff --git a/llvm/test/CodeGen/X86/bswap.ll b/llvm/test/CodeGen/X86/bswap.ll index 2a90780..89e5b3c 100644 --- a/llvm/test/CodeGen/X86/bswap.ll +++ b/llvm/test/CodeGen/X86/bswap.ll @@ -69,32 +69,27 @@ define i64 @Y(i64 %A) { define i32 @bswap_multiuse(i32 %x, i32 %y, i32* %p1, i32* %p2) nounwind { ; CHECK-LABEL: bswap_multiuse: ; CHECK: # %bb.0: -; CHECK-NEXT: pushl %edi ; CHECK-NEXT: pushl %esi ; CHECK-NEXT: movl {{[0-9]+}}(%esp), %ecx ; CHECK-NEXT: movl {{[0-9]+}}(%esp), %edx +; CHECK-NEXT: movl {{[0-9]+}}(%esp), %eax ; CHECK-NEXT: movl {{[0-9]+}}(%esp), %esi -; CHECK-NEXT: movl {{[0-9]+}}(%esp), %edi -; CHECK-NEXT: movl %edi, %eax -; CHECK-NEXT: orl %esi, %eax -; CHECK-NEXT: bswapl %edi ; CHECK-NEXT: bswapl %esi -; CHECK-NEXT: movl %edi, (%edx) -; CHECK-NEXT: movl %esi, (%ecx) ; CHECK-NEXT: bswapl %eax +; CHECK-NEXT: movl %esi, (%edx) +; CHECK-NEXT: movl %eax, (%ecx) +; CHECK-NEXT: orl %esi, %eax ; CHECK-NEXT: popl %esi -; CHECK-NEXT: popl %edi ; CHECK-NEXT: retl ; ; CHECK64-LABEL: bswap_multiuse: ; CHECK64: # %bb.0: -; CHECK64-NEXT: movl %edi, %eax -; CHECK64-NEXT: orl %esi, %eax +; CHECK64-NEXT: movl %esi, %eax ; CHECK64-NEXT: bswapl %edi -; CHECK64-NEXT: bswapl %esi -; CHECK64-NEXT: movl %edi, (%rdx) -; CHECK64-NEXT: movl %esi, (%rcx) ; CHECK64-NEXT: bswapl %eax +; CHECK64-NEXT: movl %edi, (%rdx) +; CHECK64-NEXT: movl %eax, (%rcx) +; CHECK64-NEXT: orl %edi, %eax ; CHECK64-NEXT: retq %xt = call i32 @llvm.bswap.i32(i32 %x) %yt = call i32 @llvm.bswap.i32(i32 %y)