This is an archive of the discontinued LLVM Phabricator instance.

Differential D148321

[ConstraintElimination] Add tests to check for transfering facts from sgt to ugt.
ClosedPublic

Authored by zjaffal on Apr 14 2023, 3:32 AM.

Details

Reviewers
fhahn
spatel
nikic
Commits
rG51f3781aece4: [ConstraintElimination] Add tests to check for transfering facts from sgt to…
Summary

If we have a >s b and we know that b >s 0 then we also can infer that a >u b.

Diff Detail

Unit TestsFailed

TimeTest
60,070 msx64 debian > MLIR.Examples/standalone::test.toy

Event Timeline

zjaffal created this revision.Apr 14 2023, 3:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 3:32 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
zjaffal requested review of this revision.Apr 14 2023, 3:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 3:32 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
zjaffal updated this revision to Diff 513523.Apr 14 2023, 3:43 AM

update test cases to use uge instead of ugt

zjaffal retitled this revision from [ConstraintElimination] Add tests to check for transfering facts from sgt to ugt. to [ConstraintElimination] Add tests to check for transfering facts from sgt to uge..Apr 14 2023, 3:43 AM
zjaffal edited the summary of this revision. (Show Details)
zjaffal updated this revision to Diff 513533.Apr 14 2023, 4:10 AM

change back to ugt

zjaffal retitled this revision from [ConstraintElimination] Add tests to check for transfering facts from sgt to uge. to [ConstraintElimination] Add tests to check for transfering facts from sgt to ugt..Apr 14 2023, 4:10 AM
zjaffal edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B225570: Diff 513533.Apr 14 2023, 4:41 AM
fhahn added a comment.Apr 14 2023, 5:40 AM

Thanks for adding the extra tests. A few suggestions about additional coverage inline

llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll
599

Could you add a variant where we also check icmp ugt i8 %a, 1 (negative test)

626

please also add a variant of the test where we have no info about %b's range (i.e. no check for %b).

642

please also add a test with a different negative value.

645

please don't use %t.x prefix for values that are not expected to be simplified; better just use %c. (here and below)

zjaffal updated this revision to Diff 513585.Apr 14 2023, 7:22 AM

add more tests

fhahn accepted this revision.Apr 14 2023, 7:34 AM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 14 2023, 7:34 AM
This revision was landed with ongoing or failed builds.Apr 14 2023, 7:44 AM
Closed by commit rG51f3781aece4: [ConstraintElimination] Add tests to check for transfering facts from sgt to… (authored by zjaffal). · Explain Why
This revision was automatically updated to reflect the committed changes.
zjaffal added a commit: rG51f3781aece4: [ConstraintElimination] Add tests to check for transfering facts from sgt to….
Harbormaster completed remote builds in B225614: Diff 513585.Apr 14 2023, 8:45 AM

Revision Contents

PathSize
llvm/
test/
Transforms/
ConstraintElimination/
68 lines

Diff 513533

llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll

Show First 20 Lines • Show All 590 Lines • ▼ Show 20 Linesentry:
%t.1 = icmp ugt i8 %idx, 2 %t.1 = icmp ugt i8 %idx, 2
%t.2 = icmp ugt i8 %idx, 1 %t.2 = icmp ugt i8 %idx, 1
%res.1 = xor i1 %t.1, %t.2 %res.1 = xor i1 %t.1, %t.2
%c.1 = icmp ugt i8 %idx, 3 %c.1 = icmp ugt i8 %idx, 3
%res.2 = xor i1 %res.1, %c.1 %res.2 = xor i1 %res.1, %c.1
ret i1 %res.2 ret i1 %res.2
} }
define i1 @sgt_to_ugt(i8 %a) {
fhahnUnsubmitted
Not Done
ReplyInline Actions

Could you add a variant where we also check icmp ugt i8 %a, 1 (negative test)

fhahn: Could you add a variant where we also check `icmp ugt i8 %a, 1` (negative test)
; CHECK-LABEL: @sgt_to_ugt(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[A:%.*]], 2
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: [[T_1:%.*]] = icmp ugt i8 [[A]], 2
; CHECK-NEXT: ret i1 [[T_1]]
;
entry:
%cmp = icmp sgt i8 %a, 2
call void @llvm.assume(i1 %cmp)
%t.1 = icmp ugt i8 %a, 2
ret i1 %t.1
}
define i1 @sgt_to_ugt_var(i8 %a, i8 %b) {
; CHECK-LABEL: @sgt_to_ugt_var(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[CMP_2:%.*]] = icmp sgt i8 [[B]], 0
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP_2]])
; CHECK-NEXT: [[T_1:%.*]] = icmp ugt i8 [[A]], [[B]]
; CHECK-NEXT: ret i1 [[T_1]]
;
entry:
%cmp = icmp sgt i8 %a, %b
%cmp.2 = icmp sgt i8 %b, 0
fhahnUnsubmitted
Not Done
ReplyInline Actions

please also add a variant of the test where we have no info about %b's range (i.e. no check for %b).

fhahn: please also add a variant of the test where we have no info about `%b`'s range (i.e. no check…
call void @llvm.assume(i1 %cmp)
call void @llvm.assume(i1 %cmp.2)
%t.1 = icmp ugt i8 %a, %b
ret i1 %t.1
}
define i1 @sgt_to_ugt_not(i8 %a) {
; CHECK-LABEL: @sgt_to_ugt_not(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[A:%.*]], -1
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: [[T_1:%.*]] = icmp ugt i8 [[A]], -1
; CHECK-NEXT: ret i1 [[T_1]]
;
entry:
%cmp = icmp sgt i8 %a, -1
fhahnUnsubmitted
Not Done
ReplyInline Actions

please also add a test with a different negative value.

fhahn: please also add a test with a different negative value.
call void @llvm.assume(i1 %cmp)
%t.1 = icmp ugt i8 %a, -1
ret i1 %t.1
fhahnUnsubmitted
Not Done
ReplyInline Actions

please don't use %t.x prefix for values that are not expected to be simplified; better just use %c. (here and below)

fhahn: please don't use `%t.x` prefix for values that are not expected to be simplified; better just…
}
define i1 @sgt_to_ugt_var_not(i8 %a, i8 %b) {
; CHECK-LABEL: @sgt_to_ugt_var_not(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[CMP_2:%.*]] = icmp sgt i8 [[B]], -2
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP_2]])
; CHECK-NEXT: [[T_1:%.*]] = icmp ugt i8 [[A]], [[B]]
; CHECK-NEXT: ret i1 [[T_1]]
;
entry:
%cmp = icmp sgt i8 %a, %b
%cmp.2 = icmp sgt i8 %b, -2
call void @llvm.assume(i1 %cmp)
call void @llvm.assume(i1 %cmp.2)
%t.1 = icmp ugt i8 %a, %b
ret i1 %t.1
}
define i1 @slt_first_op_known_pos(i8 %idx) { define i1 @slt_first_op_known_pos(i8 %idx) {
; CHECK-LABEL: @slt_first_op_known_pos( ; CHECK-LABEL: @slt_first_op_known_pos(
; CHECK-NEXT: entry: ; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 2, [[IDX:%.*]] ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 2, [[IDX:%.*]]
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]]) ; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: [[T_1:%.*]] = icmp ult i8 2, [[IDX]] ; CHECK-NEXT: [[T_1:%.*]] = icmp ult i8 2, [[IDX]]
; CHECK-NEXT: [[T_2:%.*]] = icmp ult i8 1, [[IDX]] ; CHECK-NEXT: [[T_2:%.*]] = icmp ult i8 1, [[IDX]]
; CHECK-NEXT: [[RES_1:%.*]] = xor i1 true, true ; CHECK-NEXT: [[RES_1:%.*]] = xor i1 true, true
Show All 37 Lines