This is an archive of the discontinued LLVM Phabricator instance.

Differential D157798

[ValueTracking] Add tests for deducing non-zero based for incoming phi-edges; NFC
ClosedPublic

Authored by goldstein.w.n on Aug 12 2023, 2:43 PM.

Details

Reviewers
nikic
RKSimon
Commits
rGfb92c0700bc5: [ValueTracking] Add tests for deducing non-zero based for incoming phi-edges…

Diff Detail

Event Timeline

goldstein.w.n created this revision.Aug 12 2023, 2:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2023, 2:43 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
goldstein.w.n requested review of this revision.Aug 12 2023, 2:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2023, 2:43 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
goldstein.w.n added a child revision: D157800: [ValueTracking] Improve analysis of knownbits from incoming phi edges..Aug 12 2023, 2:44 PM
goldstein.w.n added a parent revision: D157797: [ValueTracking] Add tests for getting knownbits from phi-edges; NFC.
Harbormaster completed remote builds in B252143: Diff 549652.Aug 12 2023, 5:26 PM
goldstein.w.n updated this revision to Diff 549674.Aug 12 2023, 6:26 PM

Rebase

goldstein.w.n updated this revision to Diff 549676.Aug 12 2023, 6:28 PM

Rebase

Harbormaster completed remote builds in B252165: Diff 549676.Aug 12 2023, 9:44 PM
nikic added inline comments.Aug 13 2023, 1:05 AM
llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll
28

ugeh...

212

Missing test for the equal successor case?

And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?

goldstein.w.n marked an inline comment as done.Aug 13 2023, 10:31 AM
goldstein.w.n added inline comments.
llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll
212

Missing test for the equal successor case?

What do you mean 'equal successor case'?

And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?

eq_non_zero2 and all the *lt/*le cases?
But added a failure case for this as well.

goldstein.w.n updated this revision to Diff 549720.Aug 13 2023, 10:31 AM

Add some more tests

Harbormaster completed remote builds in B252202: Diff 549720.Aug 13 2023, 1:07 PM
nikic added inline comments.Aug 14 2023, 3:00 AM
llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll
25

Does just br label %T not work here? I don't think the conditional is needed.

212

What do you mean 'equal successor case'?

br i1 %cmp, label %T, label %T

What do you mean 'equal successor case'?

Oh, I see. You always use br i1 %cmp, label %T, label %F, but in some cases the phi is in %T and in some in %F. Okay, that also works.

goldstein.w.n marked 2 inline comments as done.Aug 14 2023, 9:06 AM
goldstein.w.n added inline comments.
llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll
25

Switched to do that.

212

Switched to the uncond logic.

goldstein.w.n updated this revision to Diff 549986.Aug 14 2023, 9:23 AM
goldstein.w.n marked 2 inline comments as done.

Fixup br

Harbormaster completed remote builds in B252397: Diff 549986.Aug 14 2023, 3:05 PM
nikic accepted this revision.Aug 15 2023, 2:08 AM

LGTM, but I think this is still missing a test case for equal successors, something like this:

define i1 @equal_successors(i8 %x) {
entry:
  %cmp = icmp ugt i8 %x, 32
  br i1 %cmp, label %T, label %T
T:
  %v = phi i8 [ %x, %entry], [-1, %F]
  %r = icmp eq i8 %v, 0
  ret i1 %r
}
This revision is now accepted and ready to land.Aug 15 2023, 2:08 AM
goldstein.w.n updated this revision to Diff 550621.Aug 15 2023, 11:06 PM

Rebase

Harbormaster completed remote builds in B252860: Diff 550621.Aug 16 2023, 1:33 AM
Closed by commit rGfb92c0700bc5: [ValueTracking] Add tests for deducing non-zero based for incoming phi-edges… (authored by goldstein.w.n). · Explain WhyAug 22 2023, 8:59 AM
This revision was automatically updated to reflect the committed changes.
goldstein.w.n added a commit: rGfb92c0700bc5: [ValueTracking] Add tests for deducing non-zero based for incoming phi-edges….

Revision Contents

PathSize
llvm/
test/
Analysis/
ValueTracking/
211 lines

Diff 549674

llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=instsimplify < %s -S | FileCheck %s
define i1 @phi_ugt_non_zero(i8 %x) {
; CHECK-LABEL: @phi_ugt_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], 32
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[F]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
; CHECK: F:
; CHECK-NEXT: br i1 true, label [[T]], label [[F]]
;
entry:
%cmp = icmp ugt i8 %x, 32
br i1 %cmp, label %T, label %F
T:
%v = phi i8 [ %x, %entry], [-1, %F]
%r = icmp eq i8 %v, 0
ret i1 %r
F:
br i1 true, label %T, label %F
}
nikicUnsubmitted
Done
ReplyInline Actions

Does just br label %T not work here? I don't think the conditional is needed.

nikic: Does just `br label %T` not work here? I don't think the conditional is needed.
goldstein.w.nAuthorUnsubmitted
Done
ReplyInline Actions

Switched to do that.

goldstein.w.n: Switched to do that.
define i1 @phi_ugeh_non_zero(i8 %x) {
; CHECK-LABEL: @phi_ugeh_non_zero(
nikicUnsubmitted
Done
ReplyInline Actions

ugeh...

nikic: ugeh...
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp uge i8 [[X:%.*]], 32
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[F]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
; CHECK: F:
; CHECK-NEXT: br i1 true, label [[T]], label [[F]]
;
entry:
%cmp = icmp uge i8 %x, 32
br i1 %cmp, label %T, label %F
T:
%v = phi i8 [ %x, %entry], [-1, %F]
%r = icmp eq i8 %v, 0
ret i1 %r
F:
br i1 true, label %T, label %F
}
define i1 @phi_ugt_non_zero_fail(i8 %x) {
; CHECK-LABEL: @phi_ugt_non_zero_fail(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i8 [[X:%.*]], 32
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ 0, [[F]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
; CHECK: F:
; CHECK-NEXT: br i1 true, label [[T]], label [[F]]
;
entry:
%cmp = icmp ugt i8 %x, 32
br i1 %cmp, label %T, label %F
T:
%v = phi i8 [ %x, %entry], [0, %F]
%r = icmp eq i8 %v, 0
ret i1 %r
F:
br i1 true, label %T, label %F
}
define i1 @phi_ult_non_zero(i8 %x) {
; CHECK-LABEL: @phi_ult_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[X:%.*]], 123
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: br i1 true, label [[F]], label [[T]]
; CHECK: F:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[T]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
;
entry:
%cmp = icmp ult i8 %x, 123
br i1 %cmp, label %T, label %F
T:
br i1 true, label %F, label %T
F:
%v = phi i8 [ %x, %entry], [-1, %T]
%r = icmp eq i8 %v, 0
ret i1 %r
}
define i1 @phi_ule_non_zero(i8 %x) {
; CHECK-LABEL: @phi_ule_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ule i8 [[X:%.*]], 123
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: br i1 true, label [[F]], label [[T]]
; CHECK: F:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[T]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
;
entry:
%cmp = icmp ule i8 %x, 123
br i1 %cmp, label %T, label %F
T:
br i1 true, label %F, label %T
F:
%v = phi i8 [ %x, %entry], [-1, %T]
%r = icmp eq i8 %v, 0
ret i1 %r
}
define i1 @phi_ult_non_zero_fail(i8 %x) {
; CHECK-LABEL: @phi_ult_non_zero_fail(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[X:%.*]], 123
; CHECK-NEXT: [[X2:%.*]] = add i8 [[X]], [[X]]
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: br i1 true, label [[F]], label [[T]]
; CHECK: F:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X2]], [[ENTRY:%.*]] ], [ -1, [[T]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
;
entry:
%cmp = icmp ult i8 %x, 123
%x2 = add i8 %x, %x
br i1 %cmp, label %T, label %F
T:
br i1 true, label %F, label %T
F:
%v = phi i8 [ %x2, %entry], [-1, %T]
%r = icmp eq i8 %v, 0
ret i1 %r
}
define i1 @phi_ne_non_zero(i8 %x) {
; CHECK-LABEL: @phi_ne_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[X:%.*]], 0
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[F]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
; CHECK: F:
; CHECK-NEXT: br i1 true, label [[T]], label [[F]]
;
entry:
%cmp = icmp ne i8 %x, 0
br i1 %cmp, label %T, label %F
T:
%v = phi i8 [ %x, %entry], [-1, %F]
%r = icmp eq i8 %v, 0
ret i1 %r
F:
br i1 true, label %T, label %F
}
define i1 @phi_eq_non_zero(i8 %x) {
; CHECK-LABEL: @phi_eq_non_zero(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], 44
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[F]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
; CHECK: F:
; CHECK-NEXT: br i1 true, label [[T]], label [[F]]
;
entry:
%cmp = icmp eq i8 %x, 44
br i1 %cmp, label %T, label %F
T:
%v = phi i8 [ %x, %entry], [-1, %F]
%r = icmp eq i8 %v, 0
ret i1 %r
F:
br i1 true, label %T, label %F
}
define i1 @phi_eq_non_zero2(i8 %x) {
; CHECK-LABEL: @phi_eq_non_zero2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[X:%.*]], 0
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: br i1 true, label [[F]], label [[T]]
; CHECK: F:
; CHECK-NEXT: [[V:%.*]] = phi i8 [ [[X]], [[ENTRY:%.*]] ], [ -1, [[T]] ]
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[V]], 0
; CHECK-NEXT: ret i1 [[R]]
;
entry:
%cmp = icmp eq i8 %x, 0
br i1 %cmp, label %T, label %F
T:
br i1 true, label %F, label %T
F:
%v = phi i8 [ %x, %entry], [-1, %T]
%r = icmp eq i8 %v, 0
ret i1 %r
}
nikicUnsubmitted
Not Done
ReplyInline Actions

Missing test for the equal successor case?

And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?

nikic: Missing test for the equal successor case? And unless I missed it, there is no test for…
goldstein.w.nAuthorUnsubmitted
Done
ReplyInline Actions

Missing test for the equal successor case?

What do you mean 'equal successor case'?

And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?

eq_non_zero2 and all the *lt/*le cases?
But added a failure case for this as well.

goldstein.w.n: > Missing test for the equal successor case? What do you mean 'equal successor case'? > And…
nikicUnsubmitted
Done
ReplyInline Actions

What do you mean 'equal successor case'?

br i1 %cmp, label %T, label %T

What do you mean 'equal successor case'?

Oh, I see. You always use br i1 %cmp, label %T, label %F, but in some cases the phi is in %T and in some in %F. Okay, that also works.

nikic: > What do you mean 'equal successor case'? `br i1 %cmp, label %T, label %T` > What do you…
goldstein.w.nAuthorUnsubmitted
Done
ReplyInline Actions

Switched to the uncond logic.

goldstein.w.n: Switched to the uncond logic.