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

[SYCL][SCLA] Add CodeGen capabilities for sycl_ext_oneapi_private_alloca #12894

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

victor-eds
Copy link
Contributor

The sycl_ext_oneapi_private_alloca adds new functions returning a pointer to a specialization constant length SYCL private memory allocation. This commit adds codegen support for these functions.

The sycl::private_alloca function is implemented as an alias to a new __builtin_intel_sycl_alloca builtin. This is needed to guarantee the call will lower to just an alloca instruction defining the private memory allocation.

This builtin lowers to a SYCL builtin call to __builtin_sycl_unique_stable_id and a call to a new llvm.sycl.alloca intrinsic. This intrinsic receives three arguments encoding the specialization constant used as array length, a type hint argument encoding the allocation element type and the required alignment.

Note the sycl_ext_oneapi_private_alloca extension defines two functions: private_alloca and aligned_private_alloca. This commit adds codegen support only for the first signature, but already prepares support for the aligned flavor by adding an argument encoding the memory allocation alignment to the llvm.sycl.alloca intrinsic.

The intrinsic is needed as generating an alloca instruction right away would lead to optimization passes converting the size argument, which can be of any integral type, and thus difficulting lowering to a single SPIR-V OpVariable later in the pipeline.

Copy link
Contributor

@maksimsab maksimsab left a comment

Choose a reason for hiding this comment

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

Tools

Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

CFE changes look very good to me. Thank you!

Just some nits.

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaChecking.cpp Outdated Show resolved Hide resolved
clang/test/CodeGenSYCL/builtin-alloca.cpp Show resolved Hide resolved
clang/test/CodeGenSYCL/builtin-alloca.cpp Outdated Show resolved Hide resolved
clang/test/CodeGenSYCL/builtin-alloca.cpp Outdated Show resolved Hide resolved
clang/test/SemaSYCL/builtin-alloca-errors-host.cpp Outdated Show resolved Hide resolved
clang/test/SemaSYCL/builtin-alloca-errors-device.cpp Outdated Show resolved Hide resolved
@victor-eds
Copy link
Contributor Author

@premanandrao I addressed your comments. I also removed some redundant tests:

  • SemaSYCL/builtin-alloca.cpp: I was checking the host took the host definition. That does not test our new builtin, but preprocessor SYCL_DEVICE_ONLY macros. Dropped.
  • CodeGenSYCL/builtin-alloca.cpp: Dropped host test for same reasons.

Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

FE changes LGTM.

…loca`

The
[`sycl_ext_oneapi_private_alloca`](https://github.com/intel/llvm/blob/56e9067ba69809fb6ea1fd4328456ca3a009f984/sycl/doc/extensions/experimental/sycl_ext_oneapi_private_alloca.asciidoc)
adds new functions returning a pointer to a specialization constant
length SYCL private memory allocation. This commit adds codegen
support for these functions.

The `sycl::private_alloca` function is implemented as an alias to a
new `__builtin_intel_sycl_alloca` builtin. This is needed to guarantee
the call will lower to just an `alloca` instruction defining the
private memory allocation.

This builtin lowers to a SYCL builtin call to
`__builtin_sycl_unique_stable_id` and a call to a new
`llvm.sycl.alloca` intrinsic. This intrinsic receives three arguments
encoding the specialization constant used as array length, a type hint
argument encoding the allocation element type and the required
alignment.

Note the `sycl_ext_oneapi_private_alloca` extension defines two
functions: `private_alloca` and `aligned_private_alloca`. This commit
adds codegen support only for the first signature, but already
prepares support for the aligned flavor by adding an argument encoding
the memory allocation alignment to the `llvm.sycl.alloca` intrinsic.

The intrinsic is needed as generating an `alloca` instruction right
away would lead to optimization passes converting the size argument,
which can be of any integral type, and thus difficulting lowering to a
single SPIR-V `OpVariable` later in the pipeline.

Signed-off-by: Victor Perez <victor.perez@codeplay.com>
Signed-off-by: Victor Perez <victor.perez@codeplay.com>
@sommerlukas sommerlukas merged commit a105055 into intel:sycl Mar 8, 2024
13 checks passed
@victor-eds victor-eds deleted the scla-clang-work branch March 8, 2024 13:57
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.

4 participants