From 224887520959e5855f2fe5921ce60ee1154091e2 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Tue, 1 Sep 2020 20:54:12 +0100 Subject: [PATCH] [Loads] Add canReplacePointersIfEqual helper. This patch adds an initial, incomeplete and unsound implementation of canReplacePointersIfEqual to check if a pointer value A can be replaced by another pointer value B, that are deemed to be equivalent through some means (e.g. information from conditions). Note that is in general not sound to blindly replace pointers based on equality, for example if they are based on different underlying objects. LLVM's memory model is not completely settled as of now; see https://bugs.llvm.org/show_bug.cgi?id=34548 for a more detailed discussion. The initial version of canReplacePointersIfEqual only rejects a very specific case: replacing a pointer with a constant expression that is not dereferenceable. Such a replacement is problematic and can be restricted relatively easily without impacting most code. Using it to limit replacements in GVN/SCCP/CVP only results in small differences in 7 programs out of MultiSource/SPEC2000/SPEC2006 on X86 with -O3 -flto. This patch is supposed to be an initial step to improve the current situation and the helper should be made stricter in the future. But this will require careful analysis of the impact on performance. Reviewed By: aqjune Differential Revision: https://reviews.llvm.org/D85524 --- include/llvm/Analysis/Loads.h | 9 ++++++++ lib/Analysis/Loads.cpp | 20 ++++++++++++++++ unittests/Analysis/LoadsTest.cpp | 39 ++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/include/llvm/Analysis/Loads.h b/include/llvm/Analysis/Loads.h index 5665a802942..24a05610e68 100644 --- a/include/llvm/Analysis/Loads.h +++ b/include/llvm/Analysis/Loads.h @@ -155,6 +155,15 @@ Value *FindAvailablePtrLoadStore(Value *Ptr, Type *AccessTy, bool AtLeastAtomic, BasicBlock::iterator &ScanFrom, unsigned MaxInstsToScan, AAResults *AA, bool *IsLoadCSE, unsigned *NumScanedInst); + +/// Returns true if a pointer value \p A can be replace with another pointer +/// value \B if they are deemed equal through some means (e.g. information from +/// conditions). +/// NOTE: the current implementations is incomplete and unsound. It does not +/// reject all invalid cases yet, but will be made stricter in the future. In +/// particular this means returning true means unknown if replacement is safe. +bool canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL, + Instruction *CtxI); } #endif diff --git a/lib/Analysis/Loads.cpp b/lib/Analysis/Loads.cpp index e5245225d90..b81322b69bb 100644 --- a/lib/Analysis/Loads.cpp +++ b/lib/Analysis/Loads.cpp @@ -503,3 +503,23 @@ Value *llvm::FindAvailablePtrLoadStore(Value *Ptr, Type *AccessTy, // block. return nullptr; } + +bool llvm::canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL, + Instruction *CtxI) { + Type *Ty = A->getType(); + assert(Ty == B->getType() && Ty->isPointerTy() && + "values must have matching pointer types"); + + // NOTE: The checks in the function are incomplete and currently miss illegal + // cases! The current implementation is a starting point and the + // implementation should be made stricter over time. + if (auto *C = dyn_cast(B)) { + // Do not allow replacing a pointer with a constant pointer, unless it is + // either null or at least one byte is dereferenceable. + APInt OneByte(DL.getPointerTypeSizeInBits(A->getType()), 1); + return C->isNullValue() || + isDereferenceableAndAlignedPointer(B, Align(1), OneByte, DL, CtxI); + } + + return true; +} diff --git a/unittests/Analysis/LoadsTest.cpp b/unittests/Analysis/LoadsTest.cpp index 64758ebd39d..0ee1a5fa08a 100644 --- a/unittests/Analysis/LoadsTest.cpp +++ b/unittests/Analysis/LoadsTest.cpp @@ -59,3 +59,42 @@ entry: ASSERT_TRUE(CI); ASSERT_TRUE(CI->equalsInt(42)); } + +TEST(LoadsTest, CanReplacePointersIfEqual) { + LLVMContext C; + std::unique_ptr M = parseIR(C, + R"IR( +@y = common global [1 x i32] zeroinitializer, align 4 +@x = common global [1 x i32] zeroinitializer, align 4 + +declare void @use(i32*) + +define void @f(i32* %p) { + call void @use(i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0)) + call void @use(i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)) + ret void +} +)IR"); + const auto &DL = M->getDataLayout(); + auto *GV = M->getNamedValue("f"); + ASSERT_TRUE(GV); + auto *F = dyn_cast(GV); + ASSERT_TRUE(F); + + // NOTE: the implementation of canReplacePointersIfEqual is incomplete. + // Currently the only the cases it returns false for are really sound and + // returning true means unknown. + Value *P = &*F->arg_begin(); + auto InstIter = F->front().begin(); + Value *ConstDerefPtr = *cast(&*InstIter)->arg_begin(); + // ConstDerefPtr is a constant pointer that is provably de-referenceable. We + // can replace an arbitrary pointer with it. + EXPECT_TRUE(canReplacePointersIfEqual(P, ConstDerefPtr, DL, nullptr)); + + ++InstIter; + Value *ConstUnDerefPtr = *cast(&*InstIter)->arg_begin(); + // ConstUndDerefPtr is a constant pointer that is provably not + // de-referenceable. We cannot replace an arbitrary pointer with it. + EXPECT_FALSE( + canReplacePointersIfEqual(ConstDerefPtr, ConstUnDerefPtr, DL, nullptr)); +}