llvm icon indicating copy to clipboard operation
llvm copied to clipboard

Published 20 hours ago • verified publisher icon intel

[SYCL][DOC] Add extension for annotated_ptr class and its properties

Open tiwaria1 opened this issue 2 years ago • 10 comments

Introduce three new extensions:

  1. sycl_ext_oneapi_annotated_ptr Introduces a pointer wrapper class that provides a mechanism to attach compile-time constant information to a pointer in a manner that allows the compiler to reliably maintain and analyze the information.

  2. sycl_ext_intel_fpga_kernel_arg_properties Defines additional supported properties for annotated_ptr and annotated_arg classes. This commit will remove the extension sycl_ext_intel_fpga_annotated_arg_properties.

  3. sycl_ext_oneapi_kernel_arg_restrict_property Defines the restrict property for annotated_ptr and annotated_arg classes in a separate extension rather than including it in sycl_ext_intel_fpga_kernel_arg_properties so that vendors can support the property without having to support FPGA specific properties.

This commit also updates the annotated_arg extension to remove function overloads that are not needed.

tiwaria1 avatar Mar 08 '22 16:03 tiwaria1

Working on:

  • [x] Double check that extension aligns with latest template.
  • [x] Update the properties syntax based on recent changes to the design of compile-time properties.

tiwaria1 avatar Mar 08 '22 16:03 tiwaria1

@gmlueck , @GarveyJoe Please do another round of review. I have addressed your comments and added FPGA properties as a separate extension.

tiwaria1 avatar Apr 13 '22 07:04 tiwaria1

ping @intel/dpcpp-specification-reviewers

pvchupin avatar May 17 '22 04:05 pvchupin

ping https://github.com/orgs/intel/teams/dpcpp-specification-reviewers

We just had a meeting about this yesterday, so I presume the next step is that @tiwaria1 will update this PR.

gmlueck avatar May 17 '22 11:05 gmlueck

There are currently common properties defined in both annotated_ptr and annotated_arg, this will need to be cleaned up and consolidated.

tiwaria1 avatar May 27 '22 19:05 tiwaria1

There are currently common properties defined in both annotated_ptr and annotated_arg, this will need to be cleaned up and consolidated.

I have cleaned this up.

I think we should fold the changes in https://github.com/intel/llvm/pull/5656 into this change. Please let me know if anyone disagrees.

tiwaria1 avatar May 27 '22 22:05 tiwaria1

Hi @steffenlarsen all major comments have been addressed, please review this for merging. Addressing one minor review comment is remaining - I will address it today.

tiwaria1 avatar Jul 04 '22 14:07 tiwaria1

@tiwaria1 - Could you please address @jessicadavies-intel's last comment, then I'm happy with this.

steffenlarsen avatar Sep 07 '22 20:09 steffenlarsen

I'd like to take another look at this since it's been a while since my last review. I promise to do this by Thursday or Friday of this week.

gmlueck avatar Sep 07 '22 21:09 gmlueck

@tiwaria1 - Could you please address @jessicadavies-intel's last comment, then I'm happy with this.

I added a note in the property description.

tiwaria1 avatar Sep 07 '22 22:09 tiwaria1

Hi @steffenlarsen @Pennycook Please let me know your thoughts on this PR. Thanks!

broxigarchen avatar Dec 14 '22 16:12 broxigarchen