This is an archive of the discontinued LLVM Phabricator instance.

Differential D142382

[LV] Test case to show the problem with forced vec and rt-checks. NFC.
AcceptedPublic

Authored by nikolaypanchenko on Jan 23 2023, 9:51 AM.

Details

Reviewers
fhahn
ABataev
rui.zhang
Summary

The changeset contains a test that highlights the problem with forced vectorization and when number of required runtime checks exceeds some limit.
In the case of vec_foced it can be see that no overlap runtime checks were generated, which results to invalid vector code.

Diff Detail

Event Timeline

nikolaypanchenko created this revision.Jan 23 2023, 9:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 9:51 AM
nikolaypanchenko added reviewers: fhahn, ABataev, rui.zhang.Jan 23 2023, 9:51 AM
Herald added a subscriber: StephenFan. · View Herald TranscriptJan 23 2023, 9:51 AM
nikolaypanchenko added a comment.Jan 23 2023, 9:54 AM
This comment was removed by nikolaypanchenko.
nikolaypanchenko edited the summary of this revision. (Show Details)Jan 23 2023, 9:58 AM
nikolaypanchenko published this revision for review.Jan 23 2023, 10:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 10:52 AM
Herald added subscribers: llvm-commits, • pcwang-thead. · View Herald Transcript
nikolaypanchenko added a child revision: D142258: [LV] Ignore runtime checks threshold when vectorization is forced.Jan 23 2023, 10:58 AM
Harbormaster completed remote builds in B209419: Diff 491426.Jan 23 2023, 11:53 AM
fhahn accepted this revision.Jan 23 2023, 2:01 PM

LGTM, thanks! I still think it would be slightly preferable to not rely on debug output in the test and run it unconditionally, but don't have strong feelings about it.

llvm/test/Transforms/LoopVectorize/memcheck_threashold_vec_forced.ll
53

nit: IMO it would be slightly more readable to put this at the end of the function and rename it to exit. (same below)

109

Looks like the custom check lines cause this warning by the script. Perhaps remove the lines?

This revision is now accepted and ready to land.Jan 23 2023, 2:01 PM

Revision Contents

PathSize
llvm/
test/
Transforms/
LoopVectorize/
110 lines

Diff 491426

llvm/test/Transforms/LoopVectorize/memcheck_threashold_vec_forced.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=loop-vectorize -force-vector-width=4 -S -vectorize-memory-check-threshold=0 %s 2>&1 | FileCheck %s --check-prefix=CHECK
; RUN: opt -passes=loop-vectorize -force-vector-width=4 -disable-output -vectorize-memory-check-threshold=0 -debug-only=loop-vectorize %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEBUG
; REQUIRES: asserts
; CHECK-DEBUG-LABEL: LV: Checking a loop in 'vec_forced'
; CHECK-DEBUG: Executing best plan with VF=4,
define void @vec_forced(i64 %n, ptr %a, ptr %b) {
; CHECK-LABEL: @vec_forced(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[N:%.*]], 4
; CHECK-NEXT: br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
; CHECK: vector.ph:
; CHECK-NEXT: [[N_MOD_VF:%.*]] = urem i64 [[N]], 4
; CHECK-NEXT: [[N_VEC:%.*]] = sub i64 [[N]], [[N_MOD_VF]]
; CHECK-NEXT: br label [[VECTOR_BODY:%.*]]
; CHECK: vector.body:
; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
; CHECK-NEXT: [[TMP0:%.*]] = add i64 [[INDEX]], 0
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr inbounds i32, ptr [[B:%.*]], i64 [[TMP0]]
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i32 0
; CHECK-NEXT: [[WIDE_LOAD:%.*]] = load <4 x i32>, ptr [[TMP2]], align 4
; CHECK-NEXT: [[TMP3:%.*]] = add nsw <4 x i32> [[WIDE_LOAD]], <i32 1, i32 1, i32 1, i32 1>
; CHECK-NEXT: [[TMP4:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i64 [[TMP0]]
; CHECK-NEXT: [[TMP5:%.*]] = getelementptr inbounds i32, ptr [[TMP4]], i32 0
; CHECK-NEXT: store <4 x i32> [[TMP3]], ptr [[TMP5]], align 4
; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
; CHECK-NEXT: [[TMP6:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
; CHECK-NEXT: br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
; CHECK: middle.block:
; CHECK-NEXT: [[CMP_N:%.*]] = icmp eq i64 [[N]], [[N_VEC]]
; CHECK-NEXT: br i1 [[CMP_N]], label [[FOR_COND_CLEANUP_LOOPEXIT:%.*]], label [[SCALAR_PH]]
; CHECK: scalar.ph:
; CHECK-NEXT: [[BC_RESUME_VAL:%.*]] = phi i64 [ [[N_VEC]], [[MIDDLE_BLOCK]] ], [ 0, [[ENTRY:%.*]] ]
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.cond.cleanup.loopexit:
; CHECK-NEXT: ret void
; CHECK: for.body:
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[IV_NEXT:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[B]], i64 [[IV]]
; CHECK-NEXT: [[TMP7:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[TMP7]], 1
; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr inbounds i32, ptr [[A]], i64 [[IV]]
; CHECK-NEXT: store i32 [[ADD]], ptr [[ARRAYIDX2]], align 4
; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i64 [[IV]], 1
; CHECK-NEXT: [[EXITCOND_NOT:%.*]] = icmp eq i64 [[IV_NEXT]], [[N]]
; CHECK-NEXT: br i1 [[EXITCOND_NOT]], label [[FOR_COND_CLEANUP_LOOPEXIT]], label [[FOR_BODY]], !llvm.loop [[LOOP3:![0-9]+]]
;
entry:
br label %for.body
for.cond.cleanup.loopexit:
fhahnUnsubmitted
Not Done
ReplyInline Actions

nit: IMO it would be slightly more readable to put this at the end of the function and rename it to exit. (same below)

fhahn: nit: IMO it would be slightly more readable to put this at the end of the function and rename…
ret void
for.body:
%iv = phi i64 [ 0, %entry ], [ %iv.next, %for.body ]
%arrayidx = getelementptr inbounds i32, ptr %b, i64 %iv
%0 = load i32, ptr %arrayidx, align 4
%add = add nsw i32 %0, 1
%arrayidx2 = getelementptr inbounds i32, ptr %a, i64 %iv
store i32 %add, ptr %arrayidx2, align 4
%iv.next = add nuw nsw i64 %iv, 1
%exitcond.not = icmp eq i64 %iv.next, %n
br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body, !llvm.loop !9
}
; CHECK-DEBUG-LABEL: LV: Checking a loop in 'vec_not_forced'
; CHECK-DEBUG: number of checks exceeded threshold
; CHECK-DEBUG: LV: Too many memory checks needed.
define void @vec_not_forced(i64 %n, ptr %a, ptr %b) {
; CHECK-LABEL: @vec_not_forced(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.cond.cleanup.loopexit:
; CHECK-NEXT: ret void
; CHECK: for.body:
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[FOR_BODY]] ]
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[B:%.*]], i64 [[IV]]
; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[TMP0]], 1
; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr inbounds i32, ptr [[A:%.*]], i64 [[IV]]
; CHECK-NEXT: store i32 [[ADD]], ptr [[ARRAYIDX2]], align 4
; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i64 [[IV]], 1
; CHECK-NEXT: [[EXITCOND_NOT:%.*]] = icmp eq i64 [[IV_NEXT]], [[N:%.*]]
; CHECK-NEXT: br i1 [[EXITCOND_NOT]], label [[FOR_COND_CLEANUP_LOOPEXIT:%.*]], label [[FOR_BODY]]
;
entry:
br label %for.body
for.cond.cleanup.loopexit:
ret void
for.body:
%iv = phi i64 [ 0, %entry ], [ %iv.next, %for.body ]
%arrayidx = getelementptr inbounds i32, ptr %b, i64 %iv
%0 = load i32, ptr %arrayidx, align 4
%add = add nsw i32 %0, 1
%arrayidx2 = getelementptr inbounds i32, ptr %a, i64 %iv
store i32 %add, ptr %arrayidx2, align 4
%iv.next = add nuw nsw i64 %iv, 1
%exitcond.not = icmp eq i64 %iv.next, %n
br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body
}
!9 = distinct !{!9, !11}
!11 = !{!"llvm.loop.vectorize.enable", i1 true}
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
fhahnUnsubmitted
Not Done
ReplyInline Actions

Looks like the custom check lines cause this warning by the script. Perhaps remove the lines?

fhahn: Looks like the custom check lines cause this warning by the script. Perhaps remove the lines?
; CHECK-DEBUG: {{.*}}