From fd356522cd5bae1bc1b0f38971649a4ba165f463 Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Wed, 26 Aug 2015 03:02:58 +0000 Subject: [PATCH] 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 --- lib/Transforms/IPO/MergeFunctions.cpp | 7 ++---- .../MergeFunc/merge-const-ptr-and-int.ll | 20 ++++++++++++++++ .../MergeFunc/merge-different-vector-types.ll | 18 ++++++++++++++ .../MergeFunc/no-merge-ptr-different-sizes.ll | 24 +++++++++++++++++++ .../no-merge-ptr-int-different-values.ll | 23 ++++++++++++++++++ .../MergeFunc/undef-different-types.ll | 21 ++++++++++++++++ 6 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 test/Transforms/MergeFunc/merge-const-ptr-and-int.ll create mode 100644 test/Transforms/MergeFunc/merge-different-vector-types.ll create mode 100644 test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll create mode 100644 test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll create mode 100644 test/Transforms/MergeFunc/undef-different-types.ll diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp index a31a0803979..9ffd6534a65 100644 --- a/lib/Transforms/IPO/MergeFunctions.cpp +++ b/lib/Transforms/IPO/MergeFunctions.cpp @@ -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); } } diff --git a/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll b/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll new file mode 100644 index 00000000000..8c86ab1fbc3 --- /dev/null +++ b/test/Transforms/MergeFunc/merge-const-ptr-and-int.ll @@ -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 +} + diff --git a/test/Transforms/MergeFunc/merge-different-vector-types.ll b/test/Transforms/MergeFunc/merge-different-vector-types.ll new file mode 100644 index 00000000000..7696139b332 --- /dev/null +++ b/test/Transforms/MergeFunc/merge-different-vector-types.ll @@ -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> +} + +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*> +} diff --git a/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll b/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll new file mode 100644 index 00000000000..c0c6dab792d --- /dev/null +++ b/test/Transforms/MergeFunc/no-merge-ptr-different-sizes.ll @@ -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 +} diff --git a/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll b/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll new file mode 100644 index 00000000000..6bd656408ac --- /dev/null +++ b/test/Transforms/MergeFunc/no-merge-ptr-int-different-values.ll @@ -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 +} diff --git a/test/Transforms/MergeFunc/undef-different-types.ll b/test/Transforms/MergeFunc/undef-different-types.ll new file mode 100644 index 00000000000..4694146e55f --- /dev/null +++ b/test/Transforms/MergeFunc/undef-different-types.ll @@ -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 != ). +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 +} -- 2.34.1