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

[Doc] Allow llvm.ptr.annotation to specify decorations #2406

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MrSidims
Copy link
Contributor

@MrSidims MrSidims commented Mar 5, 2024

For example it allows to translate

  @.str.0 = private unnamed_addr addrspace(1) constant [13 x i8] c"{44:\224\22}\00", section "llvm.metadata"
  ...
  %ptr = alloca i32
  %ann_ptr = call ptr @llvm.ptr.annotation.p4.p1(ptr %ptr, ptr
   addrspace(1) @.str.0, ptr addrspace(1) @.str.1)

into OpDecorate Alignment 4

For example it allows to translate
  @.str.0 = private unnamed_addr addrspace(1) constant [13 x i8]
  c"{44:\224\22}\00", section "llvm.metadata"
  ...
  %ptr = alloca i32
  %ann_ptr = call ptr @llvm.ptr.annotation.p4.p1(ptr %ptr, ptr
   addrspace(1) @.str.0, ptr addrspace(1) @.str.1)

into OpDecorate Alignment 4

Signed-off-by: Sidorov, Dmitry <[email protected]>
Signed-off-by: Sidorov, Dmitry <[email protected]>
In cases when decoration parameter carried by ``llvm.ptr.annotation`` coalesces
with a value carried by another LLVM attribute, the translator is free to chose
the value depending on the meaning of the annotation. For example for alignment
we can chose the biggest alignment specified in LLVM IR:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
we can chose the biggest alignment specified in LLVM IR:
we can choose the biggest alignment specified in LLVM IR:

``llvm.ptr.annotation`` LLVM IR intrinsic. Decorations and MemberDecorations
specified in ``llvm.ptr.annotation`` must be in the second argument and must
have the format ``{X}`` or ``{X:Y}`` where ``X`` is an integer literal
representing the SPIR-V decoration identifier and ``Y`` is 1 or more arguments
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
representing the SPIR-V decoration identifier and ``Y`` is 1 or more arguments
representing the SPIR-V decoration identifier and ``Y`` is a set of 1 or more arguments


Class members and instructions returning a pointer can be decorated using the
``llvm.ptr.annotation`` LLVM IR intrinsic. Decorations and MemberDecorations
specified in ``llvm.ptr.annotation`` must be in the second argument and must
Copy link
Contributor

@asudarsa asudarsa Mar 24, 2024

Choose a reason for hiding this comment

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

what does 'must be in the second argument' mean here? Please reword if necessary. Thanks

@@ -547,6 +547,22 @@ member decorations.
None of the special requirements imposed from using the reserved names apply to
using decoration identifiers directly.

In cases when decoration parameter carried by ``llvm.ptr.annotation`` coalesces
with a value carried by another LLVM attribute, the translator is free to chose
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with a value carried by another LLVM attribute, the translator is free to chose
with a value carried by another LLVM attribute, the translator is free to choose

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

Looks good overall. Some minor nits.
Please try to address before merging. No need to wait for re-review.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants