Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

Address the atomics' macro rework: emulation runs by default #104

Merged
merged 4 commits into from
Jan 21, 2021

Conversation

AGindinson
Copy link

Signed-off-by: Artem Gindinson [email protected]

@AGindinson AGindinson force-pushed the atomic-tests branch 3 times, most recently from 151c484 to 1e7ade0 Compare January 21, 2021 10:49
@bader
Copy link

bader commented Jan 21, 2021

@AGindinson, please, do not force-push commits to PR branch!

@AGindinson
Copy link
Author

@AGindinson, please, do not force-push commits to PR branch!

Sorry, those were very minor edits. Relying on the "squash and merge" from now on.

@bader
Copy link

bader commented Jan 21, 2021

@AGindinson, please, do not force-push commits to PR branch!

Sorry, those were very minor edits. Relying on the "squash and merge" from now on.

It doesn't matter. You make reviewers miserable - while I was commenting on old commit, you removed it and GitHub doesn't let me to add a comment for non-existing commit, so I had re-do my work. It not a big deal for one comment, but it should be avoided to not lose reviewers comments in the future.

bader
bader previously approved these changes Jan 21, 2021
@bader bader changed the title Adress the atomics' macro rework: emulation runs by default Address the atomics' macro rework: emulation runs by default Jan 21, 2021
@AGindinson AGindinson requested a review from bader January 21, 2021 11:33
bader
bader previously approved these changes Jan 21, 2021
Copy link

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM.

Signed-off-by: Artem Gindinson <[email protected]>
Signed-off-by: Artem Gindinson <[email protected]>
AlexeySotkin
AlexeySotkin previously approved these changes Jan 21, 2021
bader
bader previously approved these changes Jan 21, 2021
AGindinson pushed a commit to AGindinson/llvm that referenced this pull request Jan 21, 2021
The new EXT/SPV_EXT_shader_atomic_float_add SPIR-V extension
allows us to further specialize `atomic::fetch_add()` for
floating point types. In device mode, we'll now be creating
an external call to a built-in-like `__spirv_AtomicFAddEXT()`.
This is similar to what is done for other atomic binary
instructions, e.g. the integer specialization of `fetch_add()`
being mapped onto `__spirv_AtomicIAdd()`.

Furthermore, `atomic::fetch_sub()` is also re-implemented
to use `__spirv_AtomicFAddEXT()`, the added operand being
a negation of the original one.

Tests for the feature have been finalized in
intel/llvm-test-suite#104.

Signed-off-by: Artem Gindinson <[email protected]>
Copy link

@bader bader left a comment

Choose a reason for hiding this comment

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

To clarify: XFAIL -> UNSUPPORTED to untie the merge from the feature implementation.

bader pushed a commit to intel/llvm that referenced this pull request Jan 21, 2021
The new EXT/SPV_EXT_shader_atomic_float_add SPIR-V extension
allows us to further specialize atomic::fetch_add() for
floating point types. In device mode, we'll now be creating
an external call to a built-in-like __spirv_AtomicFAddEXT().
This is similar to what is done for other atomic binary
instructions, e.g. the integer specialization of fetch_add()
being mapped onto __spirv_AtomicIAdd().

Furthermore, atomic::fetch_sub() is also re-implemented
to use __spirv_AtomicFAddEXT(), the added operand being
a negation of the original one.

The new implementation can be exposed if a dedicated macro is
defined: SYCL_USE_NATIVE_FP_ATOMICS. Otherwise, a fallback
is used, where the atomic operation is done via spinlock emulation.
At the moment of committing this, only Intel GPUs support the
"native" implementation, which relies on a SPIR-V extension.

Tests for the feature have been finalized in
intel/llvm-test-suite#104.

Signed-off-by: Artem Gindinson [email protected]
@pvchupin pvchupin merged commit 6cf85c6 into intel:intel Jan 21, 2021
AGindinson pushed a commit to AGindinson/llvm-test-suite that referenced this pull request Jan 22, 2021
A follow-up to intel#104 that will set up the infrastructure for
min/max implementation changes. The native tests will be
temporarily disabled on all targets, and then enabled in
steps (hence `UNSUPPORTED: *` is not a good choice).

Signed-off-by: Artem Gindinson <[email protected]>
bader pushed a commit that referenced this pull request Jan 24, 2021
A follow-up to #104 that will set up the infrastructure for
min/max implementation changes. The native tests will be
temporarily disabled on all targets, and then enabled in
steps (hence `UNSUPPORTED: *` is not a good choice).

Signed-off-by: Artem Gindinson <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants