Comparing operands should not require the same ValueID

Summary: When comparing basic blocks, there is an additional check that two Value*'s should have the same ID, which interferes with merging equivalent constants of different kinds (such as a ConstantInt and a ConstantPointerNull in the included testcase). The cmpValues function already ensures that the two values in each function are the same, so removing this check should not cause incorrect merging.

Also, the type comparison is redundant, based on reviewing the code and testing on the test suite and several large LTO bitcodes.

Author: jrkoenig
Reviewers: nlewycky, jfb, dschuff
Subscribers: llvm-commits
Differential revision: http://reviews.llvm.org/D12302

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@246001 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
JF Bastien 2015-08-26 03:02:58 +00:00
parent 091d6ba275
commit fd356522cd
6 changed files with 108 additions and 5 deletions

View File

@ -1082,11 +1082,8 @@ int FunctionComparator::cmpBasicBlocks(const BasicBlock *BBL,
Value *OpR = InstR->getOperand(i);
if (int Res = cmpValues(OpL, OpR))
return Res;
if (int Res = cmpNumbers(OpL->getValueID(), OpR->getValueID()))
return Res;
// TODO: Already checked in cmpOperation
if (int Res = cmpTypes(OpL->getType(), OpR->getType()))
return Res;
// cmpValues should ensure this is true.
assert(cmpTypes(OpL->getType(), OpR->getType()) == 0);
}
}

View File

@ -0,0 +1,20 @@
; RUN: opt -mergefunc -S < %s | FileCheck %s
; RUN: opt -mergefunc -S < %s | FileCheck -check-prefix=MERGE %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
; Afunc and Bfunc differ only in that one returns i64, the other a pointer.
; These should be merged.
define internal i64 @Afunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i64 @Afunc
store i32 4, i32* %P
store i32 6, i32* %Q
ret i64 0
}
define internal i64* @Bfunc(i32* %P, i32* %Q) {
; MERGE-NOT: @Bfunc
store i32 4, i32* %P
store i32 6, i32* %Q
ret i64* null
}

View File

@ -0,0 +1,18 @@
; RUN: opt -mergefunc -S < %s | FileCheck %s
; RUN: opt -mergefunc -S < %s | FileCheck -check-prefix=MERGE %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
; Merging should still work even if the values are wrapped in a vector.
define internal <2 x i64> @Mfunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal <2 x i64> @Mfunc
store i32 1, i32* %P
store i32 1, i32* %Q
ret <2 x i64> <i64 0, i64 0>
}
define internal <2 x i64*> @Nfunc(i32* %P, i32* %Q) {
; MERGE-NOT: @Nfunc
store i32 1, i32* %P
store i32 1, i32* %Q
ret <2 x i64*> <i64* null, i64* null>
}

View File

@ -0,0 +1,24 @@
; RUN: opt -mergefunc -S < %s | FileCheck %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
; These should not be merged, as the datalayout says a pointer is 64 bits. No
; sext/zext is specified, so these functions could lower differently.
define internal i32 @Ffunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i32 @Ffunc
; CHECK-NEXT: store
; CHECK-NEXT: store
; CHECK-NEXT: ret
store i32 1, i32* %P
store i32 3, i32* %Q
ret i32 0
}
define internal i64* @Gfunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i64* @Gfunc
; CHECK-NEXT: store
; CHECK-NEXT: store
; CHECK-NEXT: ret
store i32 1, i32* %P
store i32 3, i32* %Q
ret i64* null
}

View File

@ -0,0 +1,23 @@
; RUN: opt -mergefunc -S < %s | FileCheck %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
; These should not be merged, as 1 != 0.
define internal i64 @Ifunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i64 @Ifunc
; CHECK-NEXT: store
; CHECK-NEXT: store
; CHECK-NEXT: ret
store i32 10, i32* %P
store i32 10, i32* %Q
ret i64 1
}
define internal i64* @Jfunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i64* @Jfunc
; CHECK-NEXT: store
; CHECK-NEXT: store
; CHECK-NEXT: ret
store i32 10, i32* %P
store i32 10, i32* %Q
ret i64* null
}

View File

@ -0,0 +1,21 @@
; RUN: opt -mergefunc -S < %s | FileCheck %s
; RUN: opt -mergefunc -S < %s | FileCheck -check-prefix=MERGE %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
; Cfunc and Dfunc differ only in that one returns i64, the other a pointer, and
; both return undef. They should be merged. Note undef cannot be merged with
; anything else, because this implies the ordering will be inconsistent (i.e.
; -1 == undef and undef == 1, but -1 < 1, so we must have undef != <any int>).
define internal i64 @Cfunc(i32* %P, i32* %Q) {
; CHECK-LABEL: define internal i64 @Cfunc
store i32 4, i32* %P
store i32 6, i32* %Q
ret i64 undef
}
define internal i64* @Dfunc(i32* %P, i32* %Q) {
; MERGE-NOT: @Dfunc
store i32 4, i32* %P
store i32 6, i32* %Q
ret i64* undef
}