[llvm] 3542fee - [SCCP] Do not replace deref'able ptr with un-deref'able one.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 3 02:22:53 PDT 2020
Author: Florian Hahn
Date: 2020-09-03T10:22:21+01:00
New Revision: 3542feeb2077f267bff1ab98fb4bf20099f44bb8
URL: https://github.com/llvm/llvm-project/commit/3542feeb2077f267bff1ab98fb4bf20099f44bb8
DIFF: https://github.com/llvm/llvm-project/commit/3542feeb2077f267bff1ab98fb4bf20099f44bb8.diff
LOG: [SCCP] Do not replace deref'able ptr with un-deref'able one.
Currently IPSCCP (and others like CVP/GVN) blindly propagate pointer
equalities. In certain cases, that leads to dereferenceable pointers
being replaced, as in the example test case.
I think this is not allowed, as it introduces an access of an
un-dereferenceable pointer. Note that the pointer is inbounds, but one
past the last element, so it is valid, but not dereferenceable.
This patch is mostly to highlight the issue and start a discussion.
Currently it only checks for specifically looking
one-past-the-last-element pointers with array typed bases.
This causes the mis-compile outlined in
https://stackoverflow.com/questions/55754313/is-this-gcc-clang-past-one-pointer-comparison-behavior-conforming-or-non-standar
In the test case, if we replace %p with the GEP for the store, we
subsequently determine that the store and the load cannot alias, because
they are to different underlying objects.
Note that Alive2 seems to think that the replacement is valid:
https://alive2.llvm.org/ce/z/2rorhk
Reviewed By: efriedma
Differential Revision: https://reviews.llvm.org/D85332
Added:
Modified:
llvm/lib/Transforms/Scalar/SCCP.cpp
llvm/test/Transforms/SCCP/apint-bigint2.ll
llvm/test/Transforms/SCCP/indirectbr.ll
llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index 2afc778ed821..0035ae288ebb 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -31,6 +31,7 @@
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/Loads.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Analysis/ValueLattice.h"
#include "llvm/Analysis/ValueLatticeUtils.h"
@@ -177,6 +178,8 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
LLVMContext &Ctx;
public:
+ const DataLayout &getDataLayout() const { return DL; }
+
void addAnalysis(Function &F, AnalysisResultsForFn A) {
AnalysisResults.insert({&F, std::move(A)});
}
@@ -1630,6 +1633,14 @@ static bool tryToReplaceWithConstant(SCCPSolver &Solver, Value *V) {
return false;
}
+ // Do not propagate equality of a un-dereferenceable pointer.
+ // FIXME: Currently this only treats pointers one past the last element
+ // for array types. Should probably be much stricter.
+ if (Const->getType()->isPointerTy() &&
+ !canReplacePointersIfEqual(V, Const, Solver.getDataLayout(),
+ dyn_cast<Instruction>(V)))
+ return false;
+
LLVM_DEBUG(dbgs() << " Constant: " << *Const << " = " << *V << '\n');
// Replaces all of the uses of a variable with uses of the constant.
diff --git a/llvm/test/Transforms/SCCP/apint-bigint2.ll b/llvm/test/Transforms/SCCP/apint-bigint2.ll
index 8effa2181a4c..7d1a9a68372c 100644
--- a/llvm/test/Transforms/SCCP/apint-bigint2.ll
+++ b/llvm/test/Transforms/SCCP/apint-bigint2.ll
@@ -51,8 +51,10 @@ define i101 @large_aggregate_2() {
}
; CHECK-LABEL: @index_too_large
-; CHECK-NEXT: store i101* getelementptr (i101, i101* getelementptr ([6 x i101], [6 x i101]* @Y, i32 0, i32 -1), i101 9224497936761618431), i101** undef
-; CHECK-NEXT: ret void
+; CHECK-NEXT: %ptr1 = getelementptr [6 x i101], [6 x i101]* @Y, i32 0, i32 -1
+; CHECK-NEXT: %ptr2 = getelementptr i101, i101* %ptr1, i101 9224497936761618431
+; CHECK-NEXT: store i101* %ptr2, i101** undef
+; CHECK-NEXT: ret void
define void @index_too_large() {
%ptr1 = getelementptr [6 x i101], [6 x i101]* @Y, i32 0, i32 -1
%ptr2 = getelementptr i101, i101* %ptr1, i101 9224497936761618431
diff --git a/llvm/test/Transforms/SCCP/indirectbr.ll b/llvm/test/Transforms/SCCP/indirectbr.ll
index 6889282e3874..1a9ae8a128f1 100644
--- a/llvm/test/Transforms/SCCP/indirectbr.ll
+++ b/llvm/test/Transforms/SCCP/indirectbr.ll
@@ -31,7 +31,9 @@ BB1:
define void @indbrtest2() {
; CHECK-LABEL: @indbrtest2(
; CHECK-NEXT: entry:
-; CHECK-NEXT: br label [[BB1:%.*]]
+; CHECK-NEXT: [[B:%.*]] = inttoptr i64 ptrtoint (i8* blockaddress(@indbrtest2, [[BB1:%.*]]) to i64) to i8*
+; CHECK-NEXT: [[C:%.*]] = bitcast i8* [[B]] to i8*
+; CHECK-NEXT: br label [[BB1]]
; CHECK: BB1:
; CHECK-NEXT: call void @BB1_f()
; CHECK-NEXT: ret void
diff --git a/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll b/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
index 5857ce2d30b7..639e9ee76042 100644
--- a/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
+++ b/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
@@ -11,7 +11,7 @@ define i32 @eq_undereferenceable(i32* %p) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: store i32 2, i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1), align 4
+; CHECK-NEXT: store i32 2, i32* [[P]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0), align 4
@@ -65,7 +65,7 @@ define i1 @eq_undereferenceable_cmp_simp(i32* %p) {
; CHECK-NEXT: [[CMP_0:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
; CHECK-NEXT: br i1 [[CMP_0]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: store i32 2, i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1), align 4
+; CHECK-NEXT: store i32 2, i32* [[P]], align 4
; CHECK-NEXT: ret i1 true
; CHECK: if.end:
; CHECK-NEXT: [[CMP_2:%.*]] = icmp eq i32* [[P]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
More information about the llvm-commits
mailing list