This is an archive of the discontinued LLVM Phabricator instance.

Differential D146540

[SLP] Add test to check for GEP vectorization
ClosedPublic

Authored by zjaffal on Mar 21 2023, 9:21 AM.

Details

Reviewers
fhahn
Commits
rG984b46e6cc2a: [SLP] Add test to check for GEP vectorization
Summary

add a test to check for gep vectorization after the change from D144128 where the gep vectorization is dependant on the target hook prefersVectorizedAddressing()

Diff Detail

Unit TestsFailed

TimeTest
160 msx64 debian > LLVM.Transforms/SLPVectorizer/AArch64::getelementptr.ll

Event Timeline

zjaffal created this revision.Mar 21 2023, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 9:22 AM
Herald added subscribers: vporpo, ctetreau, StephenFan. · View Herald Transcript
zjaffal requested review of this revision.Mar 21 2023, 9:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 9:22 AM
Herald added subscribers: llvm-commits, • pcwang-thead. · View Herald Transcript
Harbormaster completed remote builds in B220749: Diff 507017.Mar 21 2023, 10:52 AM
fhahn requested changes to this revision.Mar 22 2023, 3:45 AM

Looks like the test is failing in the precommit checks.

llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll
6

Why is the new run line needed?

This revision now requires changes to proceed.Mar 22 2023, 3:45 AM
zjaffal updated this revision to Diff 507392.Mar 22 2023, 9:21 AM

Move function from AArch64 to be a generic test.

zjaffal updated this revision to Diff 507403.Mar 22 2023, 9:45 AM

move the file back to AArch64

Harbormaster completed remote builds in B221054: Diff 507403.Mar 22 2023, 10:55 AM
fhahn added inline comments.Mar 27 2023, 5:11 AM
llvm/test/Transforms/SLPVectorizer/AArch64/vector-getelementptr.ll
2 ↗(On Diff #507403)

this is missing an AArch64 triple, you need to add something like -mtriple=arm64-apple-ios.

4 ↗(On Diff #507403)

Please add a description to the test. The key issue is that it requires a vector GEP + extracts, but the cost is offset by being able to efficiently vectorize the rest of the tree.

43 ↗(On Diff #507403)

please don't use null in the test. e.g. loading from null like the test does is UB.

zjaffal updated this revision to Diff 508635.Mar 27 2023, 6:53 AM

rename variables, change ptr from null to function argument and specify the mtriple for opt

zjaffal marked 3 inline comments as done.Mar 27 2023, 6:54 AM
zjaffal added inline comments.
llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll
6

because -slp-threshold=-7 forces the test to vectorize I guess maybe it modifies the cost model?

Harbormaster completed remote builds in B221999: Diff 508635.Mar 27 2023, 7:58 AM
fhahn accepted this revision.Mar 27 2023, 8:35 AM

LGTM, thanks. Just a few bits that would be good to address before landing

llvm/test/Transforms/SLPVectorizer/AArch64/vector-getelementptr.ll
3 ↗(On Diff #508635)

Move this directly to the test definition, with a newline between comment and run line

5 ↗(On Diff #508635)

More consistent to use base3 instead of base_gep? All bases are used with geps

65 ↗(On Diff #508635)

It would be more compact to use gep instead of get elementary in the names.

This revision is now accepted and ready to land.Mar 27 2023, 8:35 AM
Closed by commit rG984b46e6cc2a: [SLP] Add test to check for GEP vectorization (authored by zjaffal). · Explain WhyMar 27 2023, 9:53 AM
This revision was automatically updated to reflect the committed changes.
zjaffal added a commit: rG984b46e6cc2a: [SLP] Add test to check for GEP vectorization.

Revision Contents

PathSize
llvm/
test/
Transforms/
SLPVectorizer/
AArch64/
78 lines

Diff 507017

llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll

; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=slp-vectorizer,dce,instcombine -slp-threshold=-7 -pass-remarks-output=%t < %s | FileCheck %s ; RUN: opt -S -passes=slp-vectorizer,dce,instcombine -slp-threshold=-7 -pass-remarks-output=%t < %s | FileCheck %s
; RUN: cat %t | FileCheck -check-prefix=YAML %s ; RUN: cat %t | FileCheck -check-prefix=YAML %s
; RUN: opt -S -passes='slp-vectorizer,dce,instcombine' -slp-threshold=-7 -pass-remarks-output=%t < %s | FileCheck %s ; RUN: opt -S -passes='slp-vectorizer,dce,instcombine' -slp-threshold=-7 -pass-remarks-output=%t < %s | FileCheck %s
; RUN: cat %t | FileCheck -check-prefix=YAML %s ; RUN: cat %t | FileCheck -check-prefix=YAML %s
; RUN: opt -S -passes='slp-vectorizer' | FileCheck -check-prefix=CHECK-GEP
fhahnUnsubmitted
Not Done
ReplyInline Actions

Why is the new run line needed?

fhahn: Why is the new run line needed?
zjaffalAuthorUnsubmitted
Done
ReplyInline Actions

because -slp-threshold=-7 forces the test to vectorize I guess maybe it modifies the cost model?

zjaffal: because `-slp-threshold=-7` forces the test to vectorize I guess maybe it modifies the cost…
target datalayout = "e-m:e-i32:64-i128:128-n32:64-S128" target datalayout = "e-m:e-i32:64-i128:128-n32:64-S128"
target triple = "aarch64--linux-gnu" target triple = "aarch64--linux-gnu"
; These tests check that we remove from consideration pairs of seed ; These tests check that we remove from consideration pairs of seed
; getelementptrs when they are known to have a constant difference. Such pairs ; getelementptrs when they are known to have a constant difference. Such pairs
; are likely not good candidates for vectorization since one can be computed ; are likely not good candidates for vectorization since one can be computed
; from the other. We use an unprofitable threshold to force vectorization. ; from the other. We use an unprofitable threshold to force vectorization.
▲ Show 20 Lines • Show All 384 Lines • ▼ Show 20 Lines;
%t137 = getelementptr inbounds i32, ptr %p.0, i64 %sub.8 %t137 = getelementptr inbounds i32, ptr %p.0, i64 %sub.8
%l.8 = load i32, ptr %t137, align 4 %l.8 = load i32, ptr %t137, align 4
call void @use(i32 %l.1, i32 %l.2, i32 %l.3, i32 %l.4, i32 %l.5, i32 %l.6, i32 %l.7, i32 %l.8) call void @use(i32 %l.1, i32 %l.2, i32 %l.3, i32 %l.4, i32 %l.5, i32 %l.6, i32 %l.7, i32 %l.8)
ret void ret void
} }
define void @test_i32_extend() {
; CHECK-GEP-LABEL: @test_i32_extend(
; CHECK-GEP-NEXT: bb:
; CHECK-GEP-NEXT: [[GETELEMENTPTR:%.*]] = getelementptr i32, ptr null, i64 1
; CHECK-GEP-NEXT: [[LOAD:%.*]] = load i32, ptr null, align 2
; CHECK-GEP-NEXT: [[ZEXT:%.*]] = zext i32 [[LOAD]] to i64
; CHECK-GEP-NEXT: [[GETELEMENTPTR1:%.*]] = getelementptr i32, ptr null, i64 1
; CHECK-GEP-NEXT: [[LOAD2:%.*]] = load i32, ptr null, align 2
; CHECK-GEP-NEXT: [[ZEXT3:%.*]] = zext i32 [[LOAD2]] to i64
; CHECK-GEP-NEXT: [[SUB:%.*]] = sub i64 [[ZEXT]], [[ZEXT3]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR4:%.*]] = getelementptr i32, ptr null, i64 [[SUB]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR5:%.*]] = getelementptr i32, ptr null, i64 2
; CHECK-GEP-NEXT: [[LOAD6:%.*]] = load i32, ptr [[GETELEMENTPTR]], align 2
; CHECK-GEP-NEXT: [[ZEXT7:%.*]] = zext i32 [[LOAD6]] to i64
; CHECK-GEP-NEXT: [[GETELEMENTPTR8:%.*]] = getelementptr i32, ptr null, i64 2
; CHECK-GEP-NEXT: [[LOAD9:%.*]] = load i32, ptr [[GETELEMENTPTR1]], align 2
; CHECK-GEP-NEXT: [[ZEXT10:%.*]] = zext i32 [[LOAD9]] to i64
; CHECK-GEP-NEXT: [[SUB11:%.*]] = sub i64 [[ZEXT7]], [[ZEXT10]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR12:%.*]] = getelementptr i32, ptr null, i64 [[SUB11]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR13:%.*]] = getelementptr i32, ptr null, i64 3
; CHECK-GEP-NEXT: [[LOAD14:%.*]] = load i32, ptr [[GETELEMENTPTR5]], align 2
; CHECK-GEP-NEXT: [[ZEXT15:%.*]] = zext i32 [[LOAD14]] to i64
; CHECK-GEP-NEXT: [[GETELEMENTPTR16:%.*]] = getelementptr i32, ptr null, i64 3
; CHECK-GEP-NEXT: [[LOAD17:%.*]] = load i32, ptr [[GETELEMENTPTR8]], align 2
; CHECK-GEP-NEXT: [[ZEXT18:%.*]] = zext i32 [[LOAD17]] to i64
; CHECK-GEP-NEXT: [[SUB19:%.*]] = sub i64 [[ZEXT15]], [[ZEXT18]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR20:%.*]] = getelementptr i32, ptr null, i64 [[SUB19]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR21:%.*]] = getelementptr i32, ptr null, i64 4
; CHECK-GEP-NEXT: [[LOAD22:%.*]] = load i32, ptr [[GETELEMENTPTR13]], align 2
; CHECK-GEP-NEXT: [[ZEXT23:%.*]] = zext i32 [[LOAD22]] to i64
; CHECK-GEP-NEXT: [[GETELEMENTPTR24:%.*]] = getelementptr i32, ptr null, i64 4
; CHECK-GEP-NEXT: [[LOAD25:%.*]] = load i32, ptr [[GETELEMENTPTR16]], align 2
; CHECK-GEP-NEXT: [[ZEXT26:%.*]] = zext i32 [[LOAD25]] to i64
; CHECK-GEP-NEXT: [[SUB27:%.*]] = sub i64 [[ZEXT23]], [[ZEXT26]]
; CHECK-GEP-NEXT: [[GETELEMENTPTR28:%.*]] = getelementptr i32, ptr null, i64 [[SUB27]]
; CHECK-GEP-NEXT: call void @use_4(ptr [[GETELEMENTPTR28]], ptr [[GETELEMENTPTR20]], ptr [[GETELEMENTPTR12]], ptr [[GETELEMENTPTR4]])
; CHECK-GEP-NEXT: ret void
;
bb:
%getelementptr = getelementptr i32, ptr null, i64 1
%load = load i32, ptr null, align 2
%zext = zext i32 %load to i64
%getelementptr1 = getelementptr i32, ptr null, i64 1
%load2 = load i32, ptr null, align 2
%zext3 = zext i32 %load2 to i64
%sub = sub i64 %zext, %zext3
%getelementptr4 = getelementptr i32, ptr null, i64 %sub
%getelementptr5 = getelementptr i32, ptr null, i64 2
%load6 = load i32, ptr %getelementptr, align 2
%zext7 = zext i32 %load6 to i64
%getelementptr8 = getelementptr i32, ptr null, i64 2
%load9 = load i32, ptr %getelementptr1, align 2
%zext10 = zext i32 %load9 to i64
%sub11 = sub i64 %zext7, %zext10
%getelementptr12 = getelementptr i32, ptr null, i64 %sub11
%getelementptr13 = getelementptr i32, ptr null, i64 3
%load14 = load i32, ptr %getelementptr5, align 2
%zext15 = zext i32 %load14 to i64
%getelementptr16 = getelementptr i32, ptr null, i64 3
%load17 = load i32, ptr %getelementptr8, align 2
%zext18 = zext i32 %load17 to i64
%sub19 = sub i64 %zext15, %zext18
%getelementptr20 = getelementptr i32, ptr null, i64 %sub19
%getelementptr21 = getelementptr i32, ptr null, i64 4
%load22 = load i32, ptr %getelementptr13, align 2
%zext23 = zext i32 %load22 to i64
%getelementptr24 = getelementptr i32, ptr null, i64 4
%load25 = load i32, ptr %getelementptr16, align 2
%zext26 = zext i32 %load25 to i64
%sub27 = sub i64 %zext23, %zext26
%getelementptr28 = getelementptr i32, ptr null, i64 %sub27
call void @use_4(ptr %getelementptr28, ptr %getelementptr20, ptr %getelementptr12, ptr %getelementptr4)
ret void
}
declare void @use_4(ptr, ptr, ptr, ptr)
declare void @use(i32, i32, i32, i32, i32, i32, i32, i32) declare void @use(i32, i32, i32, i32, i32, i32, i32, i32)