Skip to content

Commit a4753f5

Browse files
committed
[IR] Avoid creation of GEPs into vectors (in one place)
The method DataLayout::getGEPIndexForOffset(Type *&ElemTy, APInt &Offset) allows to generate GEP indices for a given byte-based offset. This allows to generate "natural" GEPs using the given type structure if the byte offset happens to match a nested element object. With opaque pointers and a general move towards byte-based GEPs [1], this function may be questionable in the future. This patch avoids creation of GEPs into vectors in routines that use DataLayout::getGEPIndexForOffset by not returning indices in that case. The reason is that A) GEPs into vectors have been discouraged for a long time [2], and B) that GEPs into vectors are currently broken if the element type is overaligned [1]. This is also demonstrated by a lit test where previously InstCombine replaced valid loads by poison. Note that the result of InstCombine on that test is *still* invalid, because padding bytes are assumed. Moreover, GEPs into vectors may be outright forbidden in the future [1]. [1]: https://discourse.llvm.org/t/67497 [2]: https://llvm.org/docs/GetElementPtr.html The test case is new. It will be precommitted if this patch is accepted. Differential Revision: https://reviews.llvm.org/D142146
1 parent eb197e3 commit a4753f5

File tree

3 files changed

+12
-17
lines changed

3 files changed

+12
-17
lines changed

llvm/lib/IR/DataLayout.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -940,13 +940,10 @@ std::optional<APInt> DataLayout::getGEPIndexForOffset(Type *&ElemTy,
940940
}
941941

942942
if (auto *VecTy = dyn_cast<VectorType>(ElemTy)) {
943-
ElemTy = VecTy->getElementType();
944-
unsigned ElemSizeInBits = getTypeSizeInBits(ElemTy).getFixedValue();
945-
// GEPs over non-multiple of 8 size vector elements are invalid.
946-
if (ElemSizeInBits % 8 != 0)
947-
return std::nullopt;
948-
949-
return getElementIndex(TypeSize::Fixed(ElemSizeInBits / 8), Offset);
943+
// Vector GEPs are partially broken (e.g. for overaligned element types),
944+
// and may be forbidden in the future, so avoid generating GEPs into
945+
// vectors. See https://discourse.llvm.org/t/67497
946+
return std::nullopt;
950947
}
951948

952949
if (auto *STy = dyn_cast<StructType>(ElemTy)) {

llvm/test/Transforms/InstCombine/load-gep-overalign.ll

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ define void @test_vector_load_i8() {
1212
; OVERALIGNED and NATURAL should have the same result, because the layout of vectors ignores
1313
; element type alignment, and thus the representation of @foo is the same in both cases.
1414
;
15-
; TODO: The OVERALIGNED result is incorrect.
16-
; First, for nonzero even indices, the valid load is replaced by poison.
17-
; Second, the remaining bytes at indices >= 2 are also incorrect, as apparently padding bytes
15+
; TODO: The OVERALIGNED result is incorrect, as apparently padding bytes
1816
; are assumed as they would appear in an array. In vectors, there is no padding.
1917
;
2018
; NATURAL-LABEL: @test_vector_load_i8(
@@ -31,11 +29,11 @@ define void @test_vector_load_i8() {
3129
; OVERALIGNED-LABEL: @test_vector_load_i8(
3230
; OVERALIGNED-NEXT: call void @report(i64 0, i8 1)
3331
; OVERALIGNED-NEXT: call void @report(i64 1, i8 35)
34-
; OVERALIGNED-NEXT: call void @report(i64 2, i8 poison)
32+
; OVERALIGNED-NEXT: call void @report(i64 2, i8 0)
3533
; OVERALIGNED-NEXT: call void @report(i64 3, i8 0)
36-
; OVERALIGNED-NEXT: call void @report(i64 4, i8 poison)
34+
; OVERALIGNED-NEXT: call void @report(i64 4, i8 69)
3735
; OVERALIGNED-NEXT: call void @report(i64 5, i8 103)
38-
; OVERALIGNED-NEXT: call void @report(i64 6, i8 poison)
36+
; OVERALIGNED-NEXT: call void @report(i64 6, i8 0)
3937
; OVERALIGNED-NEXT: call void @report(i64 7, i8 0)
4038
; OVERALIGNED-NEXT: ret void
4139
;

llvm/test/Transforms/LowerMatrixIntrinsics/multiply-fused-dominance.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ define void @multiply_can_hoist_multiple_insts2(ptr noalias %A, ptr %B, ptr %C)
188188
; CHECK-NEXT: [[TMP11:%.*]] = getelementptr double, ptr [[TMP3]], i64 1
189189
; CHECK-NEXT: [[COL_LOAD14:%.*]] = load <1 x double>, ptr [[TMP11]], align 8
190190
; CHECK-NEXT: [[TMP12:%.*]] = call contract <1 x double> @llvm.fmuladd.v1f64(<1 x double> [[COL_LOAD13]], <1 x double> [[COL_LOAD14]], <1 x double> [[TMP9]])
191-
; CHECK-NEXT: [[TMP13:%.*]] = getelementptr <4 x double>, ptr [[C]], i64 42, i64 1
191+
; CHECK-NEXT: [[TMP13:%.*]] = getelementptr i8, ptr [[C]], i64 1352
192192
; CHECK-NEXT: store <1 x double> [[TMP12]], ptr [[TMP13]], align 8
193193
; CHECK-NEXT: [[COL_LOAD19:%.*]] = load <1 x double>, ptr [[A]], align 8
194194
; CHECK-NEXT: [[TMP14:%.*]] = getelementptr double, ptr [[TMP3]], i64 2
@@ -199,7 +199,7 @@ define void @multiply_can_hoist_multiple_insts2(ptr noalias %A, ptr %B, ptr %C)
199199
; CHECK-NEXT: [[TMP17:%.*]] = getelementptr double, ptr [[TMP3]], i64 3
200200
; CHECK-NEXT: [[COL_LOAD25:%.*]] = load <1 x double>, ptr [[TMP17]], align 8
201201
; CHECK-NEXT: [[TMP18:%.*]] = call contract <1 x double> @llvm.fmuladd.v1f64(<1 x double> [[COL_LOAD24]], <1 x double> [[COL_LOAD25]], <1 x double> [[TMP15]])
202-
; CHECK-NEXT: [[TMP19:%.*]] = getelementptr <4 x double>, ptr [[C]], i64 42, i64 2
202+
; CHECK-NEXT: [[TMP19:%.*]] = getelementptr i8, ptr [[C]], i64 1360
203203
; CHECK-NEXT: store <1 x double> [[TMP18]], ptr [[TMP19]], align 8
204204
; CHECK-NEXT: [[TMP20:%.*]] = getelementptr double, ptr [[A]], i64 1
205205
; CHECK-NEXT: [[COL_LOAD30:%.*]] = load <1 x double>, ptr [[TMP20]], align 8
@@ -211,7 +211,7 @@ define void @multiply_can_hoist_multiple_insts2(ptr noalias %A, ptr %B, ptr %C)
211211
; CHECK-NEXT: [[TMP24:%.*]] = getelementptr double, ptr [[TMP3]], i64 3
212212
; CHECK-NEXT: [[COL_LOAD36:%.*]] = load <1 x double>, ptr [[TMP24]], align 8
213213
; CHECK-NEXT: [[TMP25:%.*]] = call contract <1 x double> @llvm.fmuladd.v1f64(<1 x double> [[COL_LOAD35]], <1 x double> [[COL_LOAD36]], <1 x double> [[TMP22]])
214-
; CHECK-NEXT: [[TMP26:%.*]] = getelementptr <4 x double>, ptr [[C]], i64 42, i64 3
214+
; CHECK-NEXT: [[TMP26:%.*]] = getelementptr i8, ptr [[C]], i64 1368
215215
; CHECK-NEXT: store <1 x double> [[TMP25]], ptr [[TMP26]], align 8
216216
; CHECK-NEXT: ret void
217217
;
@@ -248,7 +248,7 @@ define void @multiply_dont_hoist_phi(ptr noalias %A, ptr %B, ptr %C) {
248248
; CHECK-NEXT: [[TMP3:%.*]] = call contract <2 x double> @llvm.fmuladd.v2f64(<2 x double> [[COL_LOAD1]], <2 x double> [[SPLAT_SPLAT7]], <2 x double> [[TMP2]])
249249
; CHECK-NEXT: [[GEP_1:%.*]] = getelementptr <4 x double>, ptr [[C:%.*]], i64 26
250250
; CHECK-NEXT: store <2 x double> [[TMP3]], ptr [[GEP_1]], align 8
251-
; CHECK-NEXT: [[VEC_GEP14:%.*]] = getelementptr <4 x double>, ptr [[C]], i64 26, i64 2
251+
; CHECK-NEXT: [[VEC_GEP14:%.*]] = getelementptr i8, ptr [[C]], i64 848
252252
; CHECK-NEXT: store <2 x double> [[TMP1]], ptr [[VEC_GEP14]], align 8
253253
; CHECK-NEXT: ret void
254254
;

0 commit comments

Comments
 (0)