Skip to content

Commit

Permalink
[InstCombine] check alloc size in bitcast of geps fold (PR44321)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@rotateright
rotateright committed Dec 21, 2019
1 parent 19f9f37 commit 79c7fa3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
10 changes: 6 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
View file
Expand Up @@ -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
Expand Down
36 changes: 28 additions & 8 deletions llvm/test/Transforms/InstCombine/gep-vector.ll
View file
Expand Up @@ -27,26 +27,34 @@ 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]*
%gep = getelementptr [7 x i32], [7 x i32]* %arr_ptr, 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>*
%gep = getelementptr inbounds <3 x i32>, <3 x i32>* %vec_ptr, 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:%.*]]
Expand All @@ -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:%.*]]
Expand All @@ -67,30 +77,38 @@ 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)*
%gep = getelementptr [7 x i32], [7 x i32] addrspace(3)* %asc, i64 %y, i64 %z
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)*
%gep = getelementptr inbounds [7 x i32], [7 x i32] addrspace(3)* %asc, i64 %y, i64 %z
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:%.*]]
Expand All @@ -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:%.*]]
Expand Down

0 comments on commit 79c7fa3

Please sign in to comment.