[llvm] r348508 - [DAGCombiner] don't hoist logic op if operands have other uses
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 6 10:16:32 PST 2018
Author: spatel
Date: Thu Dec 6 10:16:32 2018
New Revision: 348508
URL: http://llvm.org/viewvc/llvm-project?rev=348508&view=rev
Log:
[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.
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/trunk/test/CodeGen/X86/avx512-mask-op.ll
llvm/trunk/test/CodeGen/X86/avx512-schedule.ll
llvm/trunk/test/CodeGen/X86/avx512dq-mask-op.ll
llvm/trunk/test/CodeGen/X86/bswap.ll
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=348508&r1=348507&r2=348508&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Thu Dec 6 10:16:32 2018
@@ -3715,8 +3715,8 @@ SDValue DAGCombiner::hoistLogicOpWithSam
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::hoistLogicOpWithSam
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.
Modified: llvm/trunk/test/CodeGen/X86/avx512-mask-op.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx512-mask-op.ll?rev=348508&r1=348507&r2=348508&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/avx512-mask-op.ll (original)
+++ llvm/trunk/test/CodeGen/X86/avx512-mask-op.ll Thu Dec 6 10:16:32 2018
@@ -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
Modified: llvm/trunk/test/CodeGen/X86/avx512-schedule.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx512-schedule.ll?rev=348508&r1=348507&r2=348508&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/avx512-schedule.ll (original)
+++ llvm/trunk/test/CodeGen/X86/avx512-schedule.ll Thu Dec 6 10:16:32 2018
@@ -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]
Modified: llvm/trunk/test/CodeGen/X86/avx512dq-mask-op.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx512dq-mask-op.ll?rev=348508&r1=348507&r2=348508&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/avx512dq-mask-op.ll (original)
+++ llvm/trunk/test/CodeGen/X86/avx512dq-mask-op.ll Thu Dec 6 10:16:32 2018
@@ -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>
Modified: llvm/trunk/test/CodeGen/X86/bswap.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bswap.ll?rev=348508&r1=348507&r2=348508&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/bswap.ll (original)
+++ llvm/trunk/test/CodeGen/X86/bswap.ll Thu Dec 6 10:16:32 2018
@@ -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)
More information about the llvm-commits
mailing list