[SYCL][libclc] Remangler tool for consistent mangling #4207

steffenlarsen · 2021-07-28T13:50:09Z

Libclc mangling is either done manually or done based on the function signatures if requested. However, since libclc uses OpenCL C, the primitive types are of set sizes and signedness, eg. long must be 64 bits and char must be signed.

When a target disagrees on primitive types it may use incompatible libclc functions due to inconsistent mangling between libclc and the target.

This commit adds a tool for remangling libclc bitcode files such that the mangled names match the primitive types for the target.
Additionally it creates aliases where appropriate, eg. if long is 32 bits on the target a function using long will have its mangling changed to appear as if it is using long long instead, and the old mangling will be made an alias to an existing 32-bit integer version of the function if such a function exists.

The remangler is used for the SYCL CUDA and ROCm backends to avoid having to "fake" signed char, long long and unsigned long long in libclc.

Libclc mangling is either done manually or done based on the function signatures if requested. However, since libclc uses OpenCL C, the primitive types are of set sizes and signedness, eg. long must be 64 bits and char must be signed. When a target disagrees on primitive types it may use incompatible libclc functions due to inconsistent mangling between libclc and the target. This commit adds a tool for remangling libclc bitcode files such that the mangled names match the primitive types for the target. Additionally it creates aliases where appropriate, eg. if long is 32 bits on the target a function using long will have its mangling changed to appear as if it is using long long instead, and the old mangling will be made an alias to an existing 32-bit integer version of the function if such a function exists. Signed-off-by: Steffen Larsen <[email protected]>

Signed-off-by: Steffen Larsen <[email protected]>

sycl/include/CL/__spirv/spirv_ops.hpp

dm-vodopyanov · 2021-07-29T13:13:56Z

/summary:run

elizabethandrews

FE changes look ok to me but @AaronBallman @premanandrao, can one of you take a look as well? I'm not very familiar with this

dm-vodopyanov

Temporarily approving for triggering Github checks. Will revoke the approval after that.

Revoke the approval, checks were started.

dm-vodopyanov

sycl/include/CL/__spirv/spirv_ops.hpp LGTM

AaronBallman

I have no issues with this, just a minor nit with a diagnostic quoting.

clang/include/clang/Basic/DiagnosticDriverKinds.td

Signed-off-by: Steffen Larsen <[email protected]>

AaronBallman

FE bits LGTM!

bader

LGTM, just a few minor comments.

libclc/generic/include/spirv/atomic/atomic_load.h

libclc/utils/libclc-remangler/CMakeLists.txt

libclc/utils/libclc-remangler/LibclcRemangler.cpp

Signed-off-by: Steffen Larsen <[email protected]>

bader · 2021-08-02T10:15:21Z

@mdtoguchi, @AGindinson, ping.

AGindinson

The Driver part looks good in the context of the change.

dm-vodopyanov

sycl/include/CL/__spirv/spirv_ops.hpp LGTM

dm-vodopyanov

sycl/include/CL/__spirv/spirv_ops.hpp LGTM

keryell · 2021-08-10T12:27:42Z

The introduction of this PR has a lot of information that should be present somewhere as comments in the code or is the design documentation, if it is not done already.

AidanBeltonS · 2021-08-10T13:18:34Z

The introduction of this PR has a lot of information that should be present somewhere as comments in the code or is the design documentation, if it is not done already.

The PR introduction information has not been commented or added into the design docs within this PR. I could do a separate PR adding to the design docs and commenting the relevant portions with the details in the intro. Would this work for you?

steffenlarsen · 2021-08-10T13:37:49Z

The introduction of this PR has a lot of information that should be present somewhere as comments in the code or is the design documentation, if it is not done already.

LibclcRemangler.cpp has a description early in the file to document the purpose of the tool, similar to the PR introduction. That said, if there is a suitable place to give a quick summary and refer to it I agree that it would be a good idea to do so. It is a bit of a shadow agent tool.

elizabethandrews

FE changes LGTM

Signed-off-by: Steffen Larsen <[email protected]>

…4348) Remangler tool introduced with #4207 also included simplifications to the use of `signed char` in libclc. These simplifications changed the generated SPIRV builtins in a way that causes previously passing CTS tests to fail. This PR reverts the `signed char` simplifications, preserving the remangler and the `long` simplifications. Signed-off-by: Steffen Larsen <[email protected]>

Patch adds windows support for CUDA backend. Adds general handling of Windows file paths Windows support is enabled with remangling of variables from PR #4207 as it fixes mismatch between windows 32-bit longs and default 64-bit long and handles wchar_size. Adds changes to account for MSVC's default to private classes. Fixes to unittests for windows. Signed-off-by: Steffen Larsen <[email protected]>

steffenlarsen requested review from AaronBallman, AGindinson, bader, elizabethandrews, mdtoguchi, premanandrao and a team as code owners July 28, 2021 13:50

steffenlarsen requested a review from dm-vodopyanov July 28, 2021 13:50

Fix formatting

3e1ff63

Signed-off-by: Steffen Larsen <[email protected]>

dm-vodopyanov reviewed Jul 29, 2021

View reviewed changes

sycl/include/CL/__spirv/spirv_ops.hpp Show resolved Hide resolved

elizabethandrews reviewed Jul 29, 2021

View reviewed changes

dm-vodopyanov previously approved these changes Jul 29, 2021

View reviewed changes

bader added cuda CUDA back-end libclc libclc project related issues labels Jul 29, 2021

dm-vodopyanov previously approved these changes Jul 29, 2021

View reviewed changes

AaronBallman reviewed Jul 29, 2021

View reviewed changes

clang/include/clang/Basic/DiagnosticDriverKinds.td Outdated Show resolved Hide resolved

Fix quotes in diagnostic message

cb39cb8

Signed-off-by: Steffen Larsen <[email protected]>

codeplay-sycl dismissed dm-vodopyanov’s stale review via cb39cb8 July 30, 2021 10:42

AaronBallman previously approved these changes Jul 30, 2021

View reviewed changes

bader reviewed Jul 30, 2021

View reviewed changes

Simplications

e8171e6

Signed-off-by: Steffen Larsen <[email protected]>

codeplay-sycl dismissed AaronBallman’s stale review via e8171e6 August 2, 2021 09:57

bader previously approved these changes Aug 2, 2021

View reviewed changes

AGindinson previously approved these changes Aug 2, 2021

View reviewed changes

AGindinson requested a review from AaronBallman August 2, 2021 10:22

dm-vodopyanov previously approved these changes Aug 2, 2021

View reviewed changes

AidanBeltonS dismissed stale reviews from dm-vodopyanov, AGindinson, and bader via 73b844e August 4, 2021 15:57

codeplay-sycl force-pushed the steffen/libclc-remangler branch 3 times, most recently from fb5cc9f to e8171e6 Compare August 9, 2021 09:41

Merge upstream commits to resolve conflicts

ea0d867

bader requested review from AGindinson, bader, elizabethandrews and dm-vodopyanov August 10, 2021 05:54

bader approved these changes Aug 10, 2021

View reviewed changes

bader added the hip Issues related to execution on HIP backend. label Aug 10, 2021

dm-vodopyanov approved these changes Aug 10, 2021

View reviewed changes

elizabethandrews approved these changes Aug 10, 2021

View reviewed changes

bader merged commit 66276bd into intel:sycl Aug 13, 2021

steffenlarsen added a commit to steffenlarsen/llvm that referenced this pull request Aug 16, 2021

[SYCL][libclc] Revert signed char remangling simplications (intel#4207)

bc09d8b

Signed-off-by: Steffen Larsen <[email protected]>

AidanBeltonS mentioned this pull request Aug 16, 2021

[SYCL][CUDA] Windows and MSVC support for CUDA backend #4345

Merged

steffenlarsen mentioned this pull request Aug 16, 2021

[SYCL][libclc] Revert signed char remangling simplications (#4207) #4348

Merged

This was referenced Aug 18, 2021

Reintroduce reverted signed char simplifications in libclc #4364

Closed

Clearer error message for missing libspirv library in CUDA and ROCm targets #4370

Closed

jcranmer-intel mentioned this pull request Jul 27, 2022

Rework the demangler to support recovering more pointer element types. KhronosGroup/SPIRV-LLVM-Translator#1556

Merged

[SYCL][libclc] Remangler tool for consistent mangling #4207

[SYCL][libclc] Remangler tool for consistent mangling #4207

Uh oh!

Conversation

steffenlarsen commented Jul 28, 2021

Uh oh!

Uh oh!

dm-vodopyanov commented Jul 29, 2021

Uh oh!

elizabethandrews left a comment

Choose a reason for hiding this comment

Uh oh!

dm-vodopyanov left a comment

Choose a reason for hiding this comment

Uh oh!

dm-vodopyanov left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bader commented Aug 2, 2021

Uh oh!

AGindinson left a comment

Choose a reason for hiding this comment

Uh oh!

dm-vodopyanov left a comment

Choose a reason for hiding this comment

Uh oh!

dm-vodopyanov left a comment

Choose a reason for hiding this comment

Uh oh!

keryell commented Aug 10, 2021

Uh oh!

AidanBeltonS commented Aug 10, 2021

Uh oh!

steffenlarsen commented Aug 10, 2021

Uh oh!

elizabethandrews left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!