This is an archive of the discontinued LLVM Phabricator instance.

Differential D99582

[test, LoopVectorize] Fix use of var defined in CHECK-NOT
ClosedPublic

Authored by thopre on Mar 30 2021, 5:52 AM.

Details

Reviewers
dorit
silviu.baranga
Ayal
hfinkel
fhahn
Commits
rG623475248a9d: [test, LoopVectorize] Fix use of var defined in CHECK-NOT
Summary

LLVM test Transforms/LoopVectorize/pr34681.ll tries to check for the
absence of a sequence of instructions with several CHECK-NOT with one of
those directives using a variable defined in another. However CHECK-NOT
are checked independently so that is using a variable defined in a
pattern that should not occur in the input.

This commit only checks for the absence of icmp ne 1 which rules out the
presence of the whole sequence and does not involve an undefined
variable.

Diff Detail

Unit TestsFailed

TimeTest
130 msx64 debian > cfi-devirt-lld-thinlto-x86_64.icall::bad-signature.c
100 msx64 debian > cfi-devirt-lld-x86_64.icall::bad-signature.c
120 msx64 debian > cfi-standalone-lld-thinlto-x86_64.icall::bad-signature.c
160 msx64 debian > cfi-standalone-lld-x86_64.icall::bad-signature.c

Event Timeline

thopre requested review of this revision.Mar 30 2021, 5:52 AM
thopre created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2021, 5:52 AM
Harbormaster completed remote builds in B96294: Diff 334123.Mar 30 2021, 6:32 AM
thopre mentioned this in D86222: Fix PR46880: Fail CHECK-NOT with undefined variable.Mar 30 2021, 6:40 AM
thopre updated this revision to Diff 334154.Mar 30 2021, 6:59 AM

Simplify other CHECK-NOT

Harbormaster completed remote builds in B96318: Diff 334154.Mar 30 2021, 8:04 AM
thopre added a child revision: D86222: Fix PR46880: Fail CHECK-NOT with undefined variable.Apr 5 2021, 1:57 PM
thopre added a comment.Apr 6 2021, 2:13 AM

Ping?

fhahn added a subscriber: fhahn.Apr 6 2021, 2:20 AM
fhahn added inline comments.
llvm/test/Transforms/LoopVectorize/pr34681.ll
27

what's the branch condition in this test? Could we check for the right one? Same below

thopre updated this revision to Diff 335476.Apr 6 2021, 4:17 AM
thopre marked an inline comment as done.

Keep checking each line

llvm/test/Transforms/LoopVectorize/pr34681.ll
27

We could do:

; CHECK-NOT: %ident.check = icmp ne i32 %N, 1{{(.*[[:space:]])+}}%[[TEST:[0-9]+]] = or i1 false, %ident.check{{(.*[[:space:]])+}}br i1 %[[TEST]], label %scalar.ph, label %vector.ph

but it's a lot less readable and thus more likely to have a bug in it. How about checking the absence of both icmp, or and br?

Harbormaster completed remote builds in B97277: Diff 335476.Apr 6 2021, 5:21 AM
fhahn accepted this revision.Apr 9 2021, 1:10 AM

LGTM, thanks!

llvm/test/Transforms/LoopVectorize/pr34681.ll
27

How about checking the absence of both icmp, or and br?

That sounds good, thanks!

This revision is now accepted and ready to land.Apr 9 2021, 1:10 AM
This revision was landed with ongoing or failed builds.Apr 9 2021, 2:02 AM
Closed by commit rG623475248a9d: [test, LoopVectorize] Fix use of var defined in CHECK-NOT (authored by thopre). · Explain Why
This revision was automatically updated to reflect the committed changes.
thopre added a commit: rG623475248a9d: [test, LoopVectorize] Fix use of var defined in CHECK-NOT.
thopre removed a child revision: D86222: Fix PR46880: Fail CHECK-NOT with undefined variable.Apr 9 2021, 7:07 AM

Revision Contents

PathSize
llvm/
test/
Transforms/
LoopVectorize/
8 lines

Diff 335476

llvm/test/Transforms/LoopVectorize/pr34681.ll

Show All 17 Lines
; %ident.check = icmp ne i32 %N, 1 ; %ident.check = icmp ne i32 %N, 1
; %0 = or i1 false, %ident.check ; %0 = or i1 false, %ident.check
; br i1 %0, label %scalar.ph, label %vector.ph ; br i1 %0, label %scalar.ph, label %vector.ph
; Instead the loop is vectorized with an unknown stride. ; Instead the loop is vectorized with an unknown stride.
; CHECK-LABEL: @foo1 ; CHECK-LABEL: @foo1
; CHECK: for.body.lr.ph ; CHECK: for.body.lr.ph
; CHECK-NOT: %ident.check = icmp ne i32 %N, 1 ; CHECK-NOT: %ident.check = icmp ne i32 %N, 1
; CHECK-NOT: %[[TEST:[0-9]+]] = or i1 false, %ident.check ; CHECK-NOT: %{{[0-9]+}} = or i1 false, %ident.check
; CHECK-NOT: br i1 %[[TEST]], label %scalar.ph, label %vector.ph ; CHECK-NOT: br i1 %{{[0-9]+}}, label %scalar.ph, label %vector.ph
fhahnUnsubmitted
Done
ReplyInline Actions

what's the branch condition in this test? Could we check for the right one? Same below

fhahn: what's the branch condition in this test? Could we check for the right one? Same below
thopreAuthorUnsubmitted
Done
ReplyInline Actions

We could do:

; CHECK-NOT: %ident.check = icmp ne i32 %N, 1{{(.*[[:space:]])+}}%[[TEST:[0-9]+]] = or i1 false, %ident.check{{(.*[[:space:]])+}}br i1 %[[TEST]], label %scalar.ph, label %vector.ph

but it's a lot less readable and thus more likely to have a bug in it. How about checking the absence of both icmp, or and br?

thopre: We could do: `; CHECK-NOT: %ident.check = icmp ne i32 %N, 1{{(.*[[:space:]])+}}%[[TEST:[0…
fhahnUnsubmitted
Not Done
ReplyInline Actions

How about checking the absence of both icmp, or and br?

That sounds good, thanks!

fhahn: > How about checking the absence of both icmp, or and br? That sounds good, thanks!
; CHECK: vector.ph ; CHECK: vector.ph
; CHECK: vector.body ; CHECK: vector.body
; CHECK: <4 x i32> ; CHECK: <4 x i32>
; CHECK: middle.block ; CHECK: middle.block
; CHECK: scalar.ph ; CHECK: scalar.ph
define i32 @foo1(i32 %N, i16* nocapture readnone %A, i16* nocapture readonly %B, i32 %i, i32 %j) { define i32 @foo1(i32 %N, i16* nocapture readnone %A, i16* nocapture readonly %B, i32 %i, i32 %j) {
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines
; %ident.check = icmp ne i16 %N, 1 ; %ident.check = icmp ne i16 %N, 1
; %0 = or i1 false, %ident.check ; %0 = or i1 false, %ident.check
; br i1 %0, label %scalar.ph, label %vector.ph ; br i1 %0, label %scalar.ph, label %vector.ph
; CHECK-LABEL: @foo2 ; CHECK-LABEL: @foo2
; CHECK: for.body.lr.ph ; CHECK: for.body.lr.ph
; CHECK-NOT: %ident.check = icmp ne i16 %N, 1 ; CHECK-NOT: %ident.check = icmp ne i16 %N, 1
; CHECK-NOT: %[[TEST:[0-9]+]] = or i1 false, %ident.check ; CHECK-NOT: %{{[0-9]+}} = or i1 false, %ident.check
; CHECK-NOT: br i1 %[[TEST]], label %scalar.ph, label %vector.ph ; CHECK-NOT: br i1 %{{[0-9]+}}, label %scalar.ph, label %vector.ph
; CHECK: vector.ph ; CHECK: vector.ph
; CHECK: vector.body ; CHECK: vector.body
; CHECK: <4 x i32> ; CHECK: <4 x i32>
; CHECK: middle.block ; CHECK: middle.block
; CHECK: scalar.ph ; CHECK: scalar.ph
define i32 @foo2(i16 zeroext %N, i16* nocapture readnone %A, i16* nocapture readonly %B, i32 %i, i32 %j) { define i32 @foo2(i16 zeroext %N, i16* nocapture readnone %A, i16* nocapture readonly %B, i32 %i, i32 %j) {
entry: entry:
Show All 28 Lines