Skip to content

Conversation

@EwanC
Copy link
Collaborator

@EwanC EwanC commented May 8, 2023

Rather than the user deciding whether to use the emulation mode or backend command-buffer mode, it is less error-prone to do it programmatically as a user can't select an unsupported config.

This also fixes #90 where currently the SYCL_EXT_ONEAPI_GRAPH macro isn't defined when in emulation mode.

Done by using a PI device info query in finalization. This also corresponds to a new UR device info query until the extension mechanism is decided oneapi-src/unified-runtime#458 but will be superceeded by whatever extension reporting mechanism is decided on.

The only PI backend which reports support for command-buffer implementation is Level Zero, the other backends/adapters report no support.

@EwanC EwanC added the Graph Implementation Related to DPC++ implementation and testing label May 8, 2023
@EwanC EwanC requested review from Bensuo, julianmi and reble May 8, 2023 09:25
@EwanC EwanC force-pushed the ewan/pi_query_emulation branch from 0250cb8 to db35c27 Compare May 16, 2023 13:11
@EwanC EwanC marked this pull request as ready for review May 16, 2023 13:11
Copy link
Collaborator

@julianmi julianmi 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 to me in general. One drawback I see is that this would prevent the explicit use of the emulation mode on L0, right? This might be useful for testing or comparing both approaches.

Copy link
Collaborator

@Bensuo Bensuo left a comment

Choose a reason for hiding this comment

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

LGTM

@EwanC
Copy link
Collaborator Author

EwanC commented May 17, 2023

Looks good to me in general. One drawback I see is that this would prevent the explicit use of the emulation mode on L0, right? This might be useful for testing or comparing both approaches.

Yup, I think an L0 emulation mode still does have some value. I guess we'd want to agree on whether the L0 emulation mode should be exposed to users, or it's just for us developing the code. If emulation mode on L0 is just for our own comparison & testing, then I can add an in source #ifdef to force emulation mode that can be changed, and then we recompile. Alternatively, if for an end user forcing of emulation is useful, then I can repurpose the existing CMake/configure option from "enable graphs" to "emulate graph", or something that will force the mode to be emulated.

Copy link
Owner

@reble reble left a comment

Choose a reason for hiding this comment

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

LGTM

@EwanC EwanC force-pushed the ewan/pi_query_emulation branch from a752193 to 942e98d Compare May 18, 2023 09:13
@EwanC
Copy link
Collaborator Author

EwanC commented May 18, 2023

Looks good to me in general. One drawback I see is that this would prevent the explicit use of the emulation mode on L0, right? This might be useful for testing or comparing both approaches.

Yup, I think an L0 emulation mode still does have some value. I guess we'd want to agree on whether the L0 emulation mode should be exposed to users, or it's just for us developing the code. If emulation mode on L0 is just for our own comparison & testing, then I can add an in source #ifdef to force emulation mode that can be changed, and then we recompile. Alternatively, if for an end user forcing of emulation is useful, then I can repurpose the existing CMake/configure option from "enable graphs" to "emulate graph", or something that will force the mode to be emulated.

I went with the second approach and added a FORCE_EMULATION_MODE define to the top of graph_impl.cpp. If a developer flips this from 0 to 1 and recompiles, it will force emulation node for targets with native support, i.e L0 right now.

Rather than the user deciding whether to use the emulation mode
or backend command-buffer mode, it is less error prone to do it
programmatically as a user can't select an unsupported config.

This also fixes #90 where currently
the `SYCL_EXT_ONEAPI_GRAPH` macro isn't defined when in emulation mode.

Done by using a PI device info query in finalization. This
also corresponds to a new UR device info query until the extension mechanism is
decided oneapi-src/unified-runtime#458
but will be superceeded by whatever extension reporting mechanism is
decided on.

The only PI backend which reports support for command-buffer
implementation is Level Zero, the other backends/adapters report no
support.

The vendor test macro test-e2e test is re-enabled with this change.
@EwanC EwanC force-pushed the ewan/pi_query_emulation branch from 942e98d to f8b30ab Compare May 22, 2023 06:19
@EwanC EwanC merged commit 32b9986 into sycl-graph-develop May 22, 2023
EwanC pushed a commit that referenced this pull request May 30, 2023
Remove flag from getting sarted guide as well

These were accidentally omitted from #161
EwanC pushed a commit that referenced this pull request May 31, 2023
Remove flag from getting sarted guide as well

These were accidentally omitted from #161
@Bensuo Bensuo deleted the ewan/pi_query_emulation branch August 7, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Graph Implementation Related to DPC++ implementation and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define SYCL_EXT_ONEAPI_GRAPH macro

5 participants