[llvm] 79c7fa3 - [InstCombine] check alloc size in bitcast of geps fold (PR44321)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 21 07:32:39 PST 2019
Author: Sanjay Patel
Date: 2019-12-21T10:31:21-05:00
New Revision: 79c7fa31f3aafba77862325cdc70a3ece7f71950
URL: https://github.com/llvm/llvm-project/commit/79c7fa31f3aafba77862325cdc70a3ece7f71950
DIFF: https://github.com/llvm/llvm-project/commit/79c7fa31f3aafba77862325cdc70a3ece7f71950.diff
LOG: [InstCombine] check alloc size in bitcast of geps fold (PR44321)
We missed a constraint in D44833
when folding a bitcast into a GEP with vector/array types.
If the alloc sizes specified by the datalayout don't match,
this could miscompile as shown in:
https://bugs.llvm.org/show_bug.cgi?id=44321
Differential Revision: https://reviews.llvm.org/D71771
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/gep-vector.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d423c38ce5b5..9037dd731e5d 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2191,15 +2191,17 @@ Instruction *InstCombiner::visitGetElementPtrInst(GetElementPtrInst &GEP) {
// of a bitcasted pointer to vector or array of the same dimensions:
// gep (bitcast <c x ty>* X to [c x ty]*), Y, Z --> gep X, Y, Z
// gep (bitcast [c x ty]* X to <c x ty>*), Y, Z --> gep X, Y, Z
- auto areMatchingArrayAndVecTypes = [](Type *ArrTy, Type *VecTy) {
+ auto areMatchingArrayAndVecTypes = [](Type *ArrTy, Type *VecTy,
+ const DataLayout &DL) {
return ArrTy->getArrayElementType() == VecTy->getVectorElementType() &&
- ArrTy->getArrayNumElements() == VecTy->getVectorNumElements();
+ ArrTy->getArrayNumElements() == VecTy->getVectorNumElements() &&
+ DL.getTypeAllocSize(ArrTy) == DL.getTypeAllocSize(VecTy);
};
if (GEP.getNumOperands() == 3 &&
((GEPEltType->isArrayTy() && SrcEltType->isVectorTy() &&
- areMatchingArrayAndVecTypes(GEPEltType, SrcEltType)) ||
+ areMatchingArrayAndVecTypes(GEPEltType, SrcEltType, DL)) ||
(GEPEltType->isVectorTy() && SrcEltType->isArrayTy() &&
- areMatchingArrayAndVecTypes(SrcEltType, GEPEltType)))) {
+ areMatchingArrayAndVecTypes(SrcEltType, GEPEltType, DL)))) {
// Create a new GEP here, as using `setOperand()` followed by
// `setSourceElementType()` won't actually update the type of the
diff --git a/llvm/test/Transforms/InstCombine/gep-vector.ll b/llvm/test/Transforms/InstCombine/gep-vector.ll
index 64ee29eca12b..432da0aed2dc 100644
--- a/llvm/test/Transforms/InstCombine/gep-vector.ll
+++ b/llvm/test/Transforms/InstCombine/gep-vector.ll
@@ -27,9 +27,12 @@ define <2 x i8*> @vectorindex3() {
ret <2 x i8*> %1
}
+; Negative test - datalayout's alloc size for the 2 types must match.
+
define i32* @bitcast_vec_to_array_gep(<7 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_vec_to_array_gep(
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr <7 x i32>, <7 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT: [[ARR_PTR:%.*]] = bitcast <7 x i32>* [[X:%.*]] to [7 x i32]*
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr [7 x i32], [7 x i32]* [[ARR_PTR]], i64 [[Y:%.*]], i64 [[Z:%.*]]
; CHECK-NEXT: ret i32* [[GEP]]
;
%arr_ptr = bitcast <7 x i32>* %x to [7 x i32]*
@@ -37,9 +40,12 @@ define i32* @bitcast_vec_to_array_gep(<7 x i32>* %x, i64 %y, i64 %z) {
ret i32* %gep
}
+; Negative test - datalayout's alloc size for the 2 types must match.
+
define i32* @bitcast_array_to_vec_gep([3 x i32]* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_array_to_vec_gep(
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds [3 x i32], [3 x i32]* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT: [[VEC_PTR:%.*]] = bitcast [3 x i32]* [[X:%.*]] to <3 x i32>*
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds <3 x i32>, <3 x i32>* [[VEC_PTR]], i64 [[Y:%.*]], i64 [[Z:%.*]]
; CHECK-NEXT: ret i32* [[GEP]]
;
%vec_ptr = bitcast [3 x i32]* %x to <3 x i32>*
@@ -47,6 +53,8 @@ define i32* @bitcast_array_to_vec_gep([3 x i32]* %x, i64 %y, i64 %z) {
ret i32* %gep
}
+; Sizes and types match - safe to remove bitcast.
+
define i32* @bitcast_vec_to_array_gep_matching_alloc_size(<4 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_vec_to_array_gep_matching_alloc_size(
; CHECK-NEXT: [[GEP:%.*]] = getelementptr <4 x i32>, <4 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
@@ -57,6 +65,8 @@ define i32* @bitcast_vec_to_array_gep_matching_alloc_size(<4 x i32>* %x, i64 %y,
ret i32* %gep
}
+; Sizes and types match - safe to remove bitcast.
+
define i32* @bitcast_array_to_vec_gep_matching_alloc_size([4 x i32]* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_array_to_vec_gep_matching_alloc_size(
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds [4 x i32], [4 x i32]* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
@@ -67,11 +77,14 @@ define i32* @bitcast_array_to_vec_gep_matching_alloc_size([4 x i32]* %x, i64 %y,
ret i32* %gep
}
+; Negative test - datalayout's alloc size for the 2 types must match.
+
define i32 addrspace(3)* @bitcast_vec_to_array_addrspace(<7 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_vec_to_array_addrspace(
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr <7 x i32>, <7 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
-; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast i32* [[GEP]] to i32 addrspace(3)*
-; CHECK-NEXT: ret i32 addrspace(3)* [[TMP1]]
+; CHECK-NEXT: [[ARR_PTR:%.*]] = bitcast <7 x i32>* [[X:%.*]] to [7 x i32]*
+; CHECK-NEXT: [[ASC:%.*]] = addrspacecast [7 x i32]* [[ARR_PTR]] to [7 x i32] addrspace(3)*
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr [7 x i32], [7 x i32] addrspace(3)* [[ASC]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT: ret i32 addrspace(3)* [[GEP]]
;
%arr_ptr = bitcast <7 x i32>* %x to [7 x i32]*
%asc = addrspacecast [7 x i32]* %arr_ptr to [7 x i32] addrspace(3)*
@@ -79,11 +92,14 @@ define i32 addrspace(3)* @bitcast_vec_to_array_addrspace(<7 x i32>* %x, i64 %y,
ret i32 addrspace(3)* %gep
}
+; Negative test - datalayout's alloc size for the 2 types must match.
+
define i32 addrspace(3)* @inbounds_bitcast_vec_to_array_addrspace(<7 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @inbounds_bitcast_vec_to_array_addrspace(
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds <7 x i32>, <7 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
-; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast i32* [[GEP]] to i32 addrspace(3)*
-; CHECK-NEXT: ret i32 addrspace(3)* [[TMP1]]
+; CHECK-NEXT: [[ARR_PTR:%.*]] = bitcast <7 x i32>* [[X:%.*]] to [7 x i32]*
+; CHECK-NEXT: [[ASC:%.*]] = addrspacecast [7 x i32]* [[ARR_PTR]] to [7 x i32] addrspace(3)*
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds [7 x i32], [7 x i32] addrspace(3)* [[ASC]], i64 [[Y:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT: ret i32 addrspace(3)* [[GEP]]
;
%arr_ptr = bitcast <7 x i32>* %x to [7 x i32]*
%asc = addrspacecast [7 x i32]* %arr_ptr to [7 x i32] addrspace(3)*
@@ -91,6 +107,8 @@ define i32 addrspace(3)* @inbounds_bitcast_vec_to_array_addrspace(<7 x i32>* %x,
ret i32 addrspace(3)* %gep
}
+; Sizes and types match - safe to remove bitcast.
+
define i32 addrspace(3)* @bitcast_vec_to_array_addrspace_matching_alloc_size(<4 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @bitcast_vec_to_array_addrspace_matching_alloc_size(
; CHECK-NEXT: [[GEP:%.*]] = getelementptr <4 x i32>, <4 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
@@ -103,6 +121,8 @@ define i32 addrspace(3)* @bitcast_vec_to_array_addrspace_matching_alloc_size(<4
ret i32 addrspace(3)* %gep
}
+; Sizes and types match - safe to remove bitcast.
+
define i32 addrspace(3)* @inbounds_bitcast_vec_to_array_addrspace_matching_alloc_size(<4 x i32>* %x, i64 %y, i64 %z) {
; CHECK-LABEL: @inbounds_bitcast_vec_to_array_addrspace_matching_alloc_size(
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds <4 x i32>, <4 x i32>* [[X:%.*]], i64 [[Y:%.*]], i64 [[Z:%.*]]
More information about the llvm-commits
mailing list