[llvm] r246001 - Comparing operands should not require the same ValueID

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 20:02:58 PDT 2015


Author: jfb
Date: Tue Aug 25 22:02:58 2015
New Revision: 246001

URL: http://llvm.org/viewvc/llvm-project?rev=246001&view=rev
Log:
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

Added:
    llvm/trunk/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll
    llvm/trunk/test/Transforms/MergeFunc/merge-different-vector-types.ll
    llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll
    llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll
    llvm/trunk/test/Transforms/MergeFunc/undef-different-types.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp

Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=246001&r1=246000&r2=246001&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Tue Aug 25 22:02:58 2015
@@ -1082,11 +1082,8 @@ int FunctionComparator::cmpBasicBlocks(c
         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);
       }
     }
 

Added: llvm/trunk/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll?rev=246001&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll (added)
+++ llvm/trunk/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll Tue Aug 25 22:02:58 2015
@@ -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
+}
+

Added: llvm/trunk/test/Transforms/MergeFunc/merge-different-vector-types.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/merge-different-vector-types.ll?rev=246001&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/merge-different-vector-types.ll (added)
+++ llvm/trunk/test/Transforms/MergeFunc/merge-different-vector-types.ll Tue Aug 25 22:02:58 2015
@@ -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>
+}

Added: llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll?rev=246001&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll (added)
+++ llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll Tue Aug 25 22:02:58 2015
@@ -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
+}

Added: llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll?rev=246001&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll (added)
+++ llvm/trunk/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll Tue Aug 25 22:02:58 2015
@@ -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
+}

Added: llvm/trunk/test/Transforms/MergeFunc/undef-different-types.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/undef-different-types.ll?rev=246001&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/undef-different-types.ll (added)
+++ llvm/trunk/test/Transforms/MergeFunc/undef-different-types.ll Tue Aug 25 22:02:58 2015
@@ -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
+}




More information about the llvm-commits mailing list