Skip to content
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

AMDGPU: Mark ds append/consume intrinsics with align 4 #110533

Merged

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 30, 2024

Manual says the low 2 bits of the pointer are ignored.

Manual says the low 2 bits of the pointer are ignored.
Copy link
Contributor Author

arsenm commented Sep 30, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite Graphite

@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Manual says the low 2 bits of the pointer are ignored.


Full diff: https://github.com/llvm/llvm-project/pull/110533.diff

2 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicsAMDGPU.td (+2-1)
  • (added) llvm/test/Assembler/amdgcn-intrinsic-attributes.ll (+21)
diff --git a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
index a03a92b5a97f78..50179c1ceddb47 100644
--- a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -558,7 +558,8 @@ class AMDGPUDSAppendConsumedIntrinsic : Intrinsic<
   [llvm_anyptr_ty, // LDS or GDS ptr
    llvm_i1_ty], // isVolatile
    [IntrConvergent, IntrWillReturn, IntrArgMemOnly,
-    NoCapture<ArgIndex<0>>, ImmArg<ArgIndex<1>>, IntrNoCallback, IntrNoFree],
+    Align<ArgIndex<0>, 4>, NoCapture<ArgIndex<0>>,
+    ImmArg<ArgIndex<1>>, IntrNoCallback, IntrNoFree],
    "",
    [SDNPMemOperand]
 >;
diff --git a/llvm/test/Assembler/amdgcn-intrinsic-attributes.ll b/llvm/test/Assembler/amdgcn-intrinsic-attributes.ll
new file mode 100644
index 00000000000000..3652f6a4a27e37
--- /dev/null
+++ b/llvm/test/Assembler/amdgcn-intrinsic-attributes.ll
@@ -0,0 +1,21 @@
+; REQUIRES: amdgpu-registered-target
+
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+; Test assumed alignment parameter
+
+; CHECK: declare i32 @llvm.amdgcn.ds.append.p3(ptr addrspace(3) nocapture align 4, i1 immarg) #0
+
+define i32 @ds_append(ptr addrspace(3) %ptr) {
+  %ret = call i32 @llvm.amdgcn.ds.append.p3(ptr addrspace(3) %ptr, i1 false)
+  ret i32 %ret
+}
+
+; Test assumed alignment parameter
+; CHECK: declare i32 @llvm.amdgcn.ds.consume.p3(ptr addrspace(3) nocapture align 4, i1 immarg) #0
+define i32 @ds_consume(ptr addrspace(3) %ptr) {
+  %ret = call i32 @llvm.amdgcn.ds.consume.p3(ptr addrspace(3) %ptr, i1 false)
+  ret i32 %ret
+}
+
+; CHECK: attributes #0 = { convergent nocallback nofree nounwind willreturn memory(argmem: readwrite) }

@arsenm arsenm marked this pull request as ready for review September 30, 2024 16:38
Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objection, but surely there are plenty of other load/store/atomic-like intrinsics that are missing Align<...> properties?

@arsenm
Copy link
Contributor Author

arsenm commented Oct 1, 2024

No objection, but surely there are plenty of other load/store/atomic-like intrinsics that are missing Align<...> properties?

Yes, but the ones with mangling are difficult because we can't just make it dependent on the type

@arsenm arsenm merged commit 47861fa into main Oct 1, 2024
13 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu-ds-append-consume-intrinsics-align4 branch October 1, 2024 09:57
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
Manual says the low 2 bits of the pointer are ignored.
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
Manual says the low 2 bits of the pointer are ignored.
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
Manual says the low 2 bits of the pointer are ignored.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
Manual says the low 2 bits of the pointer are ignored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants