From a42abce22562b0a0f48936ea98dd2b9efc48f1a5 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Wed, 20 Feb 2019 00:15:54 +0000 Subject: [PATCH] [GVN] Fix last crasher w/non-integral pointers Same case as for memset and memcpy, but this time for clobbering stores and loads. We still can't allow coercion to or from non-integrals, regardless of the transform. Now that I'm done the whole little sequence, it seems apparent that we'd entirely missed reasoning about clobbers in the original GVN support for non-integral pointers. My appologies, I thought we'd upstreamed all of this, but it turns out we were still carrying a downstream hack which hid all of these issues. My chanks to Cherry Zhang for helping debug. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@354407 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/VNCoercion.cpp | 20 ++++++- test/Transforms/GVN/non-integral-pointers.ll | 60 +++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/lib/Transforms/Utils/VNCoercion.cpp b/lib/Transforms/Utils/VNCoercion.cpp index 1aedce75ca6..181e53e7605 100644 --- a/lib/Transforms/Utils/VNCoercion.cpp +++ b/lib/Transforms/Utils/VNCoercion.cpp @@ -213,11 +213,22 @@ static int analyzeLoadFromClobberingWrite(Type *LoadTy, Value *LoadPtr, /// memdep query of a load that ends up being a clobbering store. int analyzeLoadFromClobberingStore(Type *LoadTy, Value *LoadPtr, StoreInst *DepSI, const DataLayout &DL) { + auto *StoredVal = DepSI->getValueOperand(); + // Cannot handle reading from store of first-class aggregate yet. - if (DepSI->getValueOperand()->getType()->isStructTy() || - DepSI->getValueOperand()->getType()->isArrayTy()) + if (StoredVal->getType()->isStructTy() || + StoredVal->getType()->isArrayTy()) return -1; + // Don't coerce non-integral pointers to integers or vice versa. + if (DL.isNonIntegralPointerType(StoredVal->getType()->getScalarType()) != + DL.isNonIntegralPointerType(LoadTy->getScalarType())) { + // Allow casts of zero values to null as a special case + auto *CI = dyn_cast(StoredVal); + if (!CI || !CI->isNullValue()) + return -1; + } + Value *StorePtr = DepSI->getPointerOperand(); uint64_t StoreSize = DL.getTypeSizeInBits(DepSI->getValueOperand()->getType()); @@ -234,6 +245,11 @@ int analyzeLoadFromClobberingLoad(Type *LoadTy, Value *LoadPtr, LoadInst *DepLI, if (DepLI->getType()->isStructTy() || DepLI->getType()->isArrayTy()) return -1; + // Don't coerce non-integral pointers to integers or vice versa. + if (DL.isNonIntegralPointerType(DepLI->getType()->getScalarType()) != + DL.isNonIntegralPointerType(LoadTy->getScalarType())) + return -1; + Value *DepPtr = DepLI->getPointerOperand(); uint64_t DepSize = DL.getTypeSizeInBits(DepLI->getType()); int R = analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, DepPtr, DepSize, DL); diff --git a/test/Transforms/GVN/non-integral-pointers.ll b/test/Transforms/GVN/non-integral-pointers.ll index fc7f74d3796..7c260639185 100644 --- a/test/Transforms/GVN/non-integral-pointers.ll +++ b/test/Transforms/GVN/non-integral-pointers.ll @@ -211,5 +211,63 @@ entry: ret i8 addrspace(4)* %ref } - declare void @llvm.memcpy.p4i8.p0i8.i64(i8 addrspace(4)* nocapture, i8* nocapture, i64, i1) nounwind + + +; Same as the neg_forward_store cases, but for non defs. +; (Pretend we wrote out the alwaysfalse idiom above.) +define i8 addrspace(4)* @neg_store_clobber(i8 addrspace(4)* addrspace(4)* %loc) { +; CHECK-LABEL: @neg_store_clobber( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[LOC_BC:%.*]] = bitcast i8 addrspace(4)* addrspace(4)* [[LOC:%.*]] to <2 x i64> addrspace(4)* +; CHECK-NEXT: store <2 x i64> , <2 x i64> addrspace(4)* [[LOC_BC]] +; CHECK-NEXT: [[LOC_OFF:%.*]] = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* [[LOC]], i64 1 +; CHECK-NEXT: [[REF:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* [[LOC_OFF]] +; CHECK-NEXT: ret i8 addrspace(4)* [[REF]] +; +entry: + %loc.bc = bitcast i8 addrspace(4)* addrspace(4)* %loc to <2 x i64> addrspace(4)* + store <2 x i64> , <2 x i64> addrspace(4)* %loc.bc + %loc.off = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc, i64 1 + %ref = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc.off + ret i8 addrspace(4)* %ref +} + +declare void @use(<2 x i64>) inaccessiblememonly + +; Same as the neg_forward_store cases, but for non defs. +; (Pretend we wrote out the alwaysfalse idiom above.) +define i8 addrspace(4)* @neg_load_clobber(i8 addrspace(4)* addrspace(4)* %loc) { +; CHECK-LABEL: @neg_load_clobber( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[LOC_BC:%.*]] = bitcast i8 addrspace(4)* addrspace(4)* [[LOC:%.*]] to <2 x i64> addrspace(4)* +; CHECK-NEXT: [[V:%.*]] = load <2 x i64>, <2 x i64> addrspace(4)* [[LOC_BC]] +; CHECK-NEXT: call void @use(<2 x i64> [[V]]) +; CHECK-NEXT: [[LOC_OFF:%.*]] = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* [[LOC]], i64 1 +; CHECK-NEXT: [[REF:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* [[LOC_OFF]] +; CHECK-NEXT: ret i8 addrspace(4)* [[REF]] +; +entry: + %loc.bc = bitcast i8 addrspace(4)* addrspace(4)* %loc to <2 x i64> addrspace(4)* + %v = load <2 x i64>, <2 x i64> addrspace(4)* %loc.bc + call void @use(<2 x i64> %v) + %loc.off = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc, i64 1 + %ref = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc.off + ret i8 addrspace(4)* %ref +} + +define i8 addrspace(4)* @store_clobber_zero(i8 addrspace(4)* addrspace(4)* %loc) { +; CHECK-LABEL: @store_clobber_zero( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[LOC_BC:%.*]] = bitcast i8 addrspace(4)* addrspace(4)* [[LOC:%.*]] to <2 x i64> addrspace(4)* +; CHECK-NEXT: store <2 x i64> zeroinitializer, <2 x i64> addrspace(4)* [[LOC_BC]] +; CHECK-NEXT: [[LOC_OFF:%.*]] = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* [[LOC]], i64 1 +; CHECK-NEXT: ret i8 addrspace(4)* null +; +entry: + %loc.bc = bitcast i8 addrspace(4)* addrspace(4)* %loc to <2 x i64> addrspace(4)* + store <2 x i64> zeroinitializer, <2 x i64> addrspace(4)* %loc.bc + %loc.off = getelementptr i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc, i64 1 + %ref = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %loc.off + ret i8 addrspace(4)* %ref +} -- 2.40.0