[llvm] r354407 - [GVN] Fix last crasher w/non-integral pointers
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 19 16:15:54 PST 2019
Author: reames
Date: Tue Feb 19 16:15:54 2019
New Revision: 354407
URL: http://llvm.org/viewvc/llvm-project?rev=354407&view=rev
Log:
[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.
Modified:
llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp
llvm/trunk/test/Transforms/GVN/non-integral-pointers.ll
Modified: llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp?rev=354407&r1=354406&r2=354407&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp Tue Feb 19 16:15:54 2019
@@ -213,11 +213,22 @@ static int analyzeLoadFromClobberingWrit
/// 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<Constant>(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 *
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);
Modified: llvm/trunk/test/Transforms/GVN/non-integral-pointers.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/non-integral-pointers.ll?rev=354407&r1=354406&r2=354407&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/non-integral-pointers.ll (original)
+++ llvm/trunk/test/Transforms/GVN/non-integral-pointers.ll Tue Feb 19 16:15:54 2019
@@ -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> <i64 4, i64 4>, <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> <i64 4, i64 4>, <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
+}
More information about the llvm-commits
mailing list