-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[AMDGPU] Match bitsin(typeof(x)) - popcnt(x) to s_bcnt0_i32 #164847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ddda647
249ee64
5bd7c7b
1030ef3
165f82d
168a5e3
9dd73e6
c3d205a
c617ef5
f5776e1
d4d3428
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,110 @@ | ||||||
| ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6 | ||||||
| ; RUN: llc -mtriple=amdgcn -mcpu=gfx900 < %s | FileCheck %s | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a GlobalISel run line to check that the patterns work there too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jayfoad, the negative tests crash Global ISel, so I can't add a check unless I break out the positive tests to a separate file. I'll do that if you like, but I think a better approach would be to file a JIRA issue to look into that. |
||||||
|
|
||||||
| define amdgpu_ps void @bcnt032_not_for_vregs(ptr addrspace(1) %out, ptr addrspace(1) %in) { | ||||||
| ; CHECK-LABEL: bcnt032_not_for_vregs: | ||||||
| ; CHECK: ; %bb.0: | ||||||
| ; CHECK-NEXT: s_lshl_b32 s0, s0, 2 | ||||||
| ; CHECK-NEXT: v_add_co_u32_e32 v2, vcc, s0, v2 | ||||||
| ; CHECK-NEXT: v_addc_co_u32_e32 v3, vcc, 0, v3, vcc | ||||||
| ; CHECK-NEXT: global_load_dword v2, v[2:3], off glc | ||||||
| ; CHECK-NEXT: s_waitcnt vmcnt(0) | ||||||
| ; CHECK-NEXT: v_bcnt_u32_b32 v2, v2, 0 | ||||||
| ; CHECK-NEXT: v_sub_u32_e32 v3, 32, v2 | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use v3 | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
| ; CHECK-NEXT: global_store_dword v[0:1], v2, off | ||||||
| ; CHECK-NEXT: s_endpgm | ||||||
| %tid = call i32 @llvm.amdgcn.workitem.id.x() | ||||||
| %gep = getelementptr inbounds i32, ptr addrspace(1) %in, i32 %tid | ||||||
| %val0 = load volatile i32, ptr addrspace(1) %gep | ||||||
| %result = call i32 @llvm.ctpop.i32(i32 %val0) nounwind readnone | ||||||
| %result2 = sub i32 32, %result | ||||||
| call void asm "; use $0", "s"(i32 %result2) | ||||||
| %cmp = icmp ne i32 %result2, 0 | ||||||
| %zext = zext i1 %cmp to i32 | ||||||
| store i32 %result, ptr addrspace(1) %out | ||||||
| ret void | ||||||
| } | ||||||
|
|
||||||
| define amdgpu_ps void @bcnt064_not_for_vregs(ptr addrspace(1) %out, ptr addrspace(1) %in) { | ||||||
| ; CHECK-LABEL: bcnt064_not_for_vregs: | ||||||
| ; CHECK: ; %bb.0: | ||||||
| ; CHECK-NEXT: s_lshl_b32 s0, s0, 2 | ||||||
| ; CHECK-NEXT: v_add_co_u32_e32 v2, vcc, s0, v2 | ||||||
| ; CHECK-NEXT: v_addc_co_u32_e32 v3, vcc, 0, v3, vcc | ||||||
| ; CHECK-NEXT: global_load_dwordx2 v[2:3], v[2:3], off glc | ||||||
| ; CHECK-NEXT: s_waitcnt vmcnt(0) | ||||||
| ; CHECK-NEXT: v_mov_b32_e32 v4, 0 | ||||||
| ; CHECK-NEXT: v_bcnt_u32_b32 v2, v2, 0 | ||||||
| ; CHECK-NEXT: v_bcnt_u32_b32 v3, v3, v2 | ||||||
| ; CHECK-NEXT: v_sub_co_u32_e32 v5, vcc, 64, v3 | ||||||
| ; CHECK-NEXT: v_subb_co_u32_e64 v6, s[0:1], 0, 0, vcc | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use v[5:6] | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
|
Comment on lines
+44
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh this is bad bug. Your SGPR constraint was lost and silently transmuted into a VGPR. Not related to this PR though, for your purposes you're just using an overly complicated test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yikes! Do you want to file the issue or should I? |
||||||
| ; CHECK-NEXT: global_store_dwordx2 v[0:1], v[3:4], off | ||||||
| ; CHECK-NEXT: s_endpgm | ||||||
| %tid = call i32 @llvm.amdgcn.workitem.id.x() | ||||||
| %gep = getelementptr inbounds i32, ptr addrspace(1) %in, i32 %tid | ||||||
| %val0 = load volatile i64, ptr addrspace(1) %gep | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. volatile load is ineligible for the VALU load to scalar load optimization. For your purposes, it is simpler to use an inreg argument to the shader calling convention rather than all of this boilerplate to load the value from memory |
||||||
| %result = call i64 @llvm.ctpop.i64(i64 %val0) nounwind readnone | ||||||
| %result2 = sub i64 64, %result | ||||||
| call void asm "; use $0", "s"(i64 %result2) | ||||||
| %cmp = icmp ne i64 %result2, 0 | ||||||
| %zext = zext i1 %cmp to i32 | ||||||
| store i64 %result, ptr addrspace(1) %out | ||||||
| ret void | ||||||
| } | ||||||
|
|
||||||
| define amdgpu_ps i32 @bcnt032_ctpop_multiple_uses(i32 inreg %val0) { | ||||||
| ; CHECK-LABEL: bcnt032_ctpop_multiple_uses: | ||||||
| ; CHECK: ; %bb.0: | ||||||
| ; CHECK-NEXT: s_bcnt1_i32_b32 s1, s0 | ||||||
| ; CHECK-NEXT: s_bcnt0_i32_b32 s0, s0 | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use s1 | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use s0 | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
| ; CHECK-NEXT: s_cselect_b64 s[0:1], -1, 0 | ||||||
| ; CHECK-NEXT: v_cndmask_b32_e64 v0, 0, 1, s[0:1] | ||||||
| ; CHECK-NEXT: v_readfirstlane_b32 s0, v0 | ||||||
| ; CHECK-NEXT: ; return to shader part epilog | ||||||
| %result = call i32 @llvm.ctpop.i32(i32 %val0) nounwind readnone | ||||||
| %result2 = sub i32 32, %result | ||||||
| call void asm "; use $0", "s"(i32 %result) | ||||||
| call void asm "; use $0", "s"(i32 %result2) | ||||||
| %cmp = icmp ne i32 %result2, 0 | ||||||
| %zext = zext i1 %cmp to i32 | ||||||
| ret i32 %zext | ||||||
| } | ||||||
|
|
||||||
| define amdgpu_ps i32 @bcnt064_ctpop_multiple_uses(i64 inreg %val0) { | ||||||
| ; CHECK-LABEL: bcnt064_ctpop_multiple_uses: | ||||||
| ; CHECK: ; %bb.0: | ||||||
| ; CHECK-NEXT: s_mov_b32 s3, 0 | ||||||
| ; CHECK-NEXT: s_bcnt1_i32_b64 s2, s[0:1] | ||||||
| ; CHECK-NEXT: s_bcnt0_i32_b64 s0, s[0:1] | ||||||
| ; CHECK-NEXT: s_mov_b32 s1, s3 | ||||||
| ; CHECK-NEXT: s_cmp_lg_u64 s[0:1], 0 | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use s[0:1] | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
| ; CHECK-NEXT: s_cselect_b64 s[0:1], -1, 0 | ||||||
| ; CHECK-NEXT: v_cndmask_b32_e64 v0, 0, 1, s[0:1] | ||||||
| ; CHECK-NEXT: v_readfirstlane_b32 s0, v0 | ||||||
| ; CHECK-NEXT: ;;#ASMSTART | ||||||
| ; CHECK-NEXT: ; use s[2:3] | ||||||
| ; CHECK-NEXT: ;;#ASMEND | ||||||
| ; CHECK-NEXT: ; return to shader part epilog | ||||||
| %result = call i64 @llvm.ctpop.i64(i64 %val0) nounwind readnone | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Don't need the callsite attributes |
||||||
| %result2 = sub i64 64, %result | ||||||
| call void asm "; use $0", "s"(i64 %result) | ||||||
| call void asm "; use $0", "s"(i64 %result2) | ||||||
| %cmp = icmp ne i64 %result2, 0 | ||||||
| %zext = zext i1 %cmp to i32 | ||||||
| ret i32 %zext | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the COPY_TO_REGCLASS really necessary? I know we had a tablegen workaround of the same shape around, but I'm not sure it's still necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arsenm I have no clue. I put it there because I was aping similar code in that file that seemed to use it when copying from 32-bit to 64-bit. I can remove it and see if it still works?