From 9d85eff56ac8fbe8f3b9a287ecb95c3c3a371cdd Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Fri, 17 Oct 2014 11:56:00 +0000 Subject: [PATCH] [DSE] Remove no-data-layout-only type-based overlap checking DSE's overlap checking contained special logic, used only when no DataLayout was available, which inferred a complete overwrite when the pointee types were equal. This logic seems fine for regular loads/stores, but does not work for memcpy and friends. Instead of fixing this, I'm just removing it. Philosophically, transformations should not contain enhanced behavior used only when data layout is lacking (data layout should be strictly additive), and maintaining these rarely-tested code paths seems not worthwhile at this stage. Credit to Aliaksei Zasenka for the bug report and the diagnosis. The test case (slightly reduced from that provided by Aliaksei) replaces the original contents of test/Transforms/DeadStoreElimination/no-targetdata.ll -- a few other tests have been updated to have a data layout. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220035 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Scalar/DeadStoreElimination.cpp | 9 +----- test/Analysis/TypeBasedAliasAnalysis/dse.ll | 1 + .../DeadStoreElimination/const-pointers.ll | 1 + .../DeadStoreElimination/inst-limits.ll | 1 + .../DeadStoreElimination/no-targetdata.ll | 30 +++++++++++-------- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/Transforms/Scalar/DeadStoreElimination.cpp b/lib/Transforms/Scalar/DeadStoreElimination.cpp index 3af8ee7546f..a1ddc00da5b 100644 --- a/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -356,15 +356,8 @@ static OverwriteResult isOverwrite(const AliasAnalysis::Location &Later, // If we don't know the sizes of either access, then we can't do a // comparison. if (Later.Size == AliasAnalysis::UnknownSize || - Earlier.Size == AliasAnalysis::UnknownSize) { - // If we have no DataLayout information around, then the size of the store - // is inferrable from the pointee type. If they are the same type, then - // we know that the store is safe. - if (DL == nullptr && Later.Ptr->getType() == Earlier.Ptr->getType()) - return OverwriteComplete; - + Earlier.Size == AliasAnalysis::UnknownSize) return OverwriteUnknown; - } // Make sure that the Later size is >= the Earlier size. if (Later.Size >= Earlier.Size) diff --git a/test/Analysis/TypeBasedAliasAnalysis/dse.ll b/test/Analysis/TypeBasedAliasAnalysis/dse.ll index bcf1f2c5275..9032fadb1ef 100644 --- a/test/Analysis/TypeBasedAliasAnalysis/dse.ll +++ b/test/Analysis/TypeBasedAliasAnalysis/dse.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -tbaa -basicaa -dse -S | FileCheck %s +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" ; DSE should make use of TBAA. diff --git a/test/Transforms/DeadStoreElimination/const-pointers.ll b/test/Transforms/DeadStoreElimination/const-pointers.ll index c90d824b34c..3e772d79c70 100644 --- a/test/Transforms/DeadStoreElimination/const-pointers.ll +++ b/test/Transforms/DeadStoreElimination/const-pointers.ll @@ -1,4 +1,5 @@ ; RUN: opt -basicaa -dse -S < %s | FileCheck %s +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" %t = type { i32 } diff --git a/test/Transforms/DeadStoreElimination/inst-limits.ll b/test/Transforms/DeadStoreElimination/inst-limits.ll index c6e527e7753..3d78bb5c316 100644 --- a/test/Transforms/DeadStoreElimination/inst-limits.ll +++ b/test/Transforms/DeadStoreElimination/inst-limits.ll @@ -1,4 +1,5 @@ ; RUN: opt -S -dse < %s | FileCheck %s +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" ; If there are two stores to the same location, DSE should be able to remove ; the first store if the two stores are separated by no more than 98 diff --git a/test/Transforms/DeadStoreElimination/no-targetdata.ll b/test/Transforms/DeadStoreElimination/no-targetdata.ll index c0c7c58d4ea..25395330bf3 100644 --- a/test/Transforms/DeadStoreElimination/no-targetdata.ll +++ b/test/Transforms/DeadStoreElimination/no-targetdata.ll @@ -1,15 +1,21 @@ ; RUN: opt -basicaa -dse -S < %s | FileCheck %s -declare void @test1f() - -define void @test1(i32* noalias %p) { - store i32 1, i32* %p - call void @test1f() - store i32 2, i32 *%p - ret void -; CHECK-LABEL: define void @test1( -; CHECK-NOT: store -; CHECK-NEXT: call void -; CHECK-NEXT: store i32 2 -; CHECK-NEXT: ret void +declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i32, i1) nounwind + +define void @fn(i8* nocapture %buf) #0 { +entry: + +; We would not eliminate the first memcpy with data layout, and we should not +; eliminate it without data layout. +; CHECK-LABEL: @fn +; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i64 +; CHECK: tail call void @llvm.memcpy.p0i8.p0i8.i64 +; CHECK: ret void + + %arrayidx = getelementptr i8* %buf, i64 18 + tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %arrayidx, i8* %buf, i64 18, i32 1, i1 false) + store i8 1, i8* %arrayidx, align 1 + tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %buf, i8* %arrayidx, i64 18, i32 1, i1 false) + ret void } + -- 2.34.1