This is an archive of the discontinued LLVM Phabricator instance.

llvm/test/Transforms/InstCombine/cttz.ll
31	It would be good to also test the ctlz case here. Possibly with a vector type, so we have at least one test that checks this transform with a vector zero.

Sorry, I didn't notice the patch stack before commenting in D60846.

LGTM - but I agree with the earlier comment about adding 1 more test for extra coverage.

This revision is now accepted and ready to land.Apr 22 2019, 9:01 AM

Probably also a good time to request commit access, see https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access for instructions.

In D60932#1474266, @nikic wrote:

Probably also a good time to request commit access, see https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access for instructions.

Will do!

llvm/test/Transforms/InstCombine/cttz.ll
31	+1 sounds good

dlrobertson marked an inline comment as not done.Apr 24 2019, 5:24 PM

dlrobertson added inline comments.

llvm/test/Transforms/InstCombine/cttz.ll
31	After looking at this, I'm not sure how this would work. The input value `x` would be a vector, but you can not `br` on a vector of `i1`s

nikic added inline comments.Apr 24 2019, 11:49 PM

llvm/test/Transforms/InstCombine/cttz.ll
31	That's a good point! I guess vectors are indeed not subject to this transform, at least not without additional handling (we'd have to recognize an "all elements zero" pattern for the branch condition.) In that case it might make sense to add an early exit for vector types in the main patch to avoid unnecessary walking the uses.

lebedev.ri added a subscriber: lebedev.ri.Apr 24 2019, 11:55 PM

lebedev.ri added inline comments.

llvm/test/Transforms/InstCombine/cttz.ll
31	%t0 = bitcast <8 x i1> to i8 icmp eq i8 %t0, 0

nikic added inline comments.Apr 25 2019, 12:05 AM

llvm/test/Transforms/InstCombine/cttz.ll
31	@lebedev.ri Thanks! I'm not sure whether it's worthwhile to add handling for this, as it would require walking uses three levels deep (uses of the original value, uses of the bitcast, uses of the icmp). It wouldn't hurt to add a test for it though, even if it'll not be handled by this patch.

dlrobertson marked an inline comment as done.Apr 25 2019, 4:38 AM

dlrobertson added inline comments.

llvm/test/Transforms/InstCombine/cttz.ll
31	%t0 = bitcast <8 x i1> to i8 icmp eq i8 %t0, 0 awesome! I'll add a test with this In that case it might make sense to add an early exit for vector types Good point. I'll add that

Closed by commit rL359267: [NFC] Add baseline tests for int isKnownNonZero (authored by dlrobertson). · Explain WhyApr 25 2019, 7:55 PM

This revision was automatically updated to reflect the committed changes.

I followed the https://llvm.org/docs/Phabricator.html#committing-a-change section for the commit. If I made a mistake, please let me know.

Revision Contents

Path

Size

llvm/

test/

Transforms/

InstCombine/

cttz.ll

31 lines

InstSimplify/

known-non-zero.ll

104 lines

Diff 195958

llvm/test/Transforms/InstCombine/cttz.ll

This file was added.

				; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
				; RUN: opt < %s -instcombine -S \| FileCheck %s

				declare i64 @llvm.cttz.i64(i64, i1)

				define i32 @test0(i64 %x) {
				; CHECK-LABEL: @test0(
				; CHECK-NEXT: start:
				; CHECK-NEXT: [[C:%.]] = icmp eq i64 [[X:%.]], 0
				; CHECK-NEXT: br i1 [[C]], label [[EXIT:%.]], label [[NON_ZERO:%.]]
				; CHECK: non_zero:
				; CHECK-NEXT: [[CTZ:%.*]] = call i64 @llvm.cttz.i64(i64 [[X]], i1 false), !range !0
				; CHECK-NEXT: [[CTZ32:%.*]] = trunc i64 [[CTZ]] to i32
				; CHECK-NEXT: br label [[EXIT]]
				; CHECK: exit:
				; CHECK-NEXT: [[RES:%.]] = phi i32 [ [[CTZ32]], [[NON_ZERO]] ], [ 0, [[START:%.]] ]
				; CHECK-NEXT: ret i32 [[RES]]
				;
				start:
				%c = icmp eq i64 %x, 0
				br i1 %c, label %exit, label %non_zero

				non_zero:
				%ctz = call i64 @llvm.cttz.i64(i64 %x, i1 false)
				%ctz32 = trunc i64 %ctz to i32
				br label %exit

				exit:
				%res = phi i32 [ %ctz32, %non_zero ], [ 0, %start ]
				ret i32 %res
				}
				nikicUnsubmitted Not Done Reply Inline Actions It would be good to also test the ctlz case here. Possibly with a vector type, so we have at least one test that checks this transform with a vector zero. nikic: It would be good to also test the ctlz case here. Possibly with a vector type, so we have at…
				dlrobertsonAuthorUnsubmitted Not Done Reply Inline Actions +1 sounds good dlrobertson: +1 sounds good
				dlrobertsonAuthorUnsubmitted Not Done Reply Inline Actions After looking at this, I'm not sure how this would work. The input value `x` would be a vector, but you can not `br` on a vector of `i1`s dlrobertson: After looking at this, I'm not sure how this would work. The input value `x` would be a vector…
				nikicUnsubmitted Not Done Reply Inline Actions That's a good point! I guess vectors are indeed not subject to this transform, at least not without additional handling (we'd have to recognize an "all elements zero" pattern for the branch condition.) In that case it might make sense to add an early exit for vector types in the main patch to avoid unnecessary walking the uses. nikic: That's a good point! I guess vectors are indeed not subject to this transform, at least not…
				lebedev.riUnsubmitted Not Done Reply Inline Actions %t0 = bitcast <8 x i1> to i8 icmp eq i8 %t0, 0 lebedev.ri: %t0 = bitcast <8 x i1> to i8 icmp eq i8 %t0, 0
				nikicUnsubmitted Not Done Reply Inline Actions @lebedev.ri Thanks! I'm not sure whether it's worthwhile to add handling for this, as it would require walking uses three levels deep (uses of the original value, uses of the bitcast, uses of the icmp). It wouldn't hurt to add a test for it though, even if it'll not be handled by this patch. nikic: @lebedev.ri Thanks! I'm not sure whether it's worthwhile to add handling for this, as it would…
				dlrobertsonAuthorUnsubmitted Done Reply Inline Actions %t0 = bitcast <8 x i1> to i8 icmp eq i8 %t0, 0 awesome! I'll add a test with this In that case it might make sense to add an early exit for vector types Good point. I'll add that dlrobertson: > %t0 = bitcast <8 x i1> to i8 > icmp eq i8 %t0, 0 awesome! I'll add a test with this > In…

llvm/test/Transforms/InstSimplify/known-non-zero.ll

This file was added.

				; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
				; RUN: opt < %s -instsimplify -S \| FileCheck %s

				define i64 @test0(i64 %x) {
				; CHECK-LABEL: @test0(
				; CHECK-NEXT: start:
				; CHECK-NEXT: [[A:%.]] = icmp eq i64 [[X:%.]], 0
				; CHECK-NEXT: br i1 [[A]], label [[EXIT:%.]], label [[NON_ZERO:%.]]
				; CHECK: non_zero:
				; CHECK-NEXT: [[B:%.*]] = icmp eq i64 [[X]], 0
				; CHECK-NEXT: br i1 [[B]], label [[UNREACHABLE:%.*]], label [[EXIT]]
				; CHECK: unreachable:
				; CHECK-NEXT: br label [[EXIT]]
				; CHECK: exit:
				; CHECK-NEXT: [[C:%.]] = phi i64 [ 0, [[START:%.]] ], [ 1, [[NON_ZERO]] ], [ 2, [[UNREACHABLE]] ]
				; CHECK-NEXT: ret i64 [[C]]
				;
				start:
				%a = icmp eq i64 %x, 0
				br i1 %a, label %exit, label %non_zero

				non_zero:
				%b = icmp eq i64 %x, 0
				br i1 %b, label %unreachable, label %exit

				unreachable:
				br label %exit

				exit:
				%c = phi i64 [ 0, %start ], [ 1, %non_zero ], [ 2, %unreachable ]
				ret i64 %c
				}

				define i64 @test1(i64 %x) {
				; CHECK-LABEL: @test1(
				; CHECK-NEXT: start:
				; CHECK-NEXT: [[A:%.]] = icmp eq i64 [[X:%.]], 0
				; CHECK-NEXT: br i1 [[A]], label [[EXIT:%.]], label [[NON_ZERO:%.]]
				; CHECK: non_zero:
				; CHECK-NEXT: [[B:%.*]] = icmp ugt i64 [[X]], 0
				; CHECK-NEXT: br i1 [[B]], label [[EXIT]], label [[UNREACHABLE:%.*]]
				; CHECK: unreachable:
				; CHECK-NEXT: br label [[EXIT]]
				; CHECK: exit:
				; CHECK-NEXT: [[C:%.]] = phi i64 [ 0, [[START:%.]] ], [ [[X]], [[NON_ZERO]] ], [ 0, [[UNREACHABLE]] ]
				; CHECK-NEXT: ret i64 [[C]]
				;
				start:
				%a = icmp eq i64 %x, 0
				br i1 %a, label %exit, label %non_zero

				non_zero:
				%b = icmp ugt i64 %x, 0
				br i1 %b, label %exit, label %unreachable

				unreachable:
				br label %exit

				exit:
				%c = phi i64 [ 0, %start ], [ %x, %non_zero ], [ 0, %unreachable ]
				ret i64 %c
				}

				define i1 @test2(i64 %x, i1 %y) {
				; CHECK-LABEL: @test2(
				; CHECK-NEXT: start:
				; CHECK-NEXT: [[A:%.]] = icmp eq i64 [[X:%.]], 0
				; CHECK-NEXT: br i1 [[A]], label [[EXIT:%.]], label [[NON_ZERO:%.]]
				; CHECK: non_zero:
				; CHECK-NEXT: br i1 [[Y:%.]], label [[ONE:%.]], label [[TWO:%.*]]
				; CHECK: one:
				; CHECK-NEXT: br label [[MAINBLOCK:%.*]]
				; CHECK: two:
				; CHECK-NEXT: br label [[MAINBLOCK]]
				; CHECK: mainblock:
				; CHECK-NEXT: [[P:%.*]] = phi i64 [ [[X]], [[ONE]] ], [ 42, [[TWO]] ]
				; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[P]], 0
				; CHECK-NEXT: br label [[EXIT]]
				; CHECK: exit:
				; CHECK-NEXT: [[RES:%.]] = phi i1 [ [[CMP]], [[MAINBLOCK]] ], [ true, [[START:%.]] ]
				; CHECK-NEXT: ret i1 [[RES]]
				;
				start:
				%a = icmp eq i64 %x, 0
				br i1 %a, label %exit, label %non_zero

				non_zero:
				br i1 %y, label %one, label %two

				one:
				br label %mainblock

				two:
				br label %mainblock

				mainblock:
				%p = phi i64 [ %x, %one ], [ 42, %two ]
				%cmp = icmp eq i64 %p, 0
				br label %exit

				exit:
				%res = phi i1 [ %cmp, %mainblock ], [ 1, %start ]
				ret i1 %res
				}

This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Add baseline tests for int isKnownNonZeroClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 195958

llvm/test/Transforms/InstCombine/cttz.ll

llvm/test/Transforms/InstSimplify/known-non-zero.ll

[NFC] Add baseline tests for int isKnownNonZero
ClosedPublic