[SYCL] Define SYCL_DISABLE_FALLBACK_ASSERT macro

[elizabethandrews]

Define SYCL_DISABLE_FALLBACK_ASSERT for FPGA

Signed-off-by: Elizabeth Andrews elizabeth.andrews@intel.com

[smanna12]

LGTM

[bader]

Tagging @s-kanaev.
There is similar patch disabling fallback assert for all targets by default - #4694.
If the plan is to merge #4694, I think we can avoid defining this macro for FPGA target.
@s-kanaev, am I right?

[elizabethandrews]

Unless we want a case where we enable fallback assert by default but keep it disabled for FPGA target, I assume we don't need this macro.

[s-kanaev]

Fallback assert is disabled by default due high impact issues being reported recently. Still, there's a plan to enable assert back. FPGA is an exception here as having one more kernel in device image impacts them more than GPU or CPU.

[elizabethandrews]

Ok. Thank you for clarifying. @bader, I think that means we want to keep the macro? At the moment, this will have no effect but once fallback assert is enabled, the macro will keep it disables for FPGA

[bader]

#4694 renames macro.
I don't know if it make sense to proceed with this patch. It's not clear when fallback asserts will be enabled by default and whether this patch will work. According to my understanding, "enabling back" won't be as simple as just revert #4694.

[bader]

@s-kanaev, what is overall plan for fallback assert? Are we going to commit #4824 and close #4694?

[s-kanaev]

Absolutely, #4694 is going to be closed. #4824 and #4780 are going to be committed. There were two types of issues:

• enabling assertions for interop kernels
• impact of assertions on FPGA
  The former is fixed in [SYCL] Disable fallback assert for interop kernels #4712.
  The latter is fixed with help of [SYCL] Define SYCL_DISABLE_FALLBACK_ASSERT macro #4824 and [SYCL] Disable submission of AssertInfoCopier for FPGA #4780.

[bader]

I think the second issue impacts non-FPGA devices as well. AFAIK, additional assert-specific device code is emitted for all devices even though assert is not used. Is it right? If so, what is the plan to address the overhead associated with the additional code?

[s-kanaev]

AFAIK, additional assert-specific device code is emitted for all devices even though assert is not used. Is it right?

Yes.
To overcome this impact I need to do some refactoring which is a bit stuck at limitations of sycl-post-link tool.
The limitation could be handled with employing of __sycl_kernel attribute, though, it requires for the function to be template which isn't acceptable at the moment. Also, @AlexeySachkov can't approve the w/a in sycl-post-link I presented to him.
The overall idea of refactoring can be caught with changes of devicelib in #4987.

[bader]

Please, follow https://chris.beams.io/posts/git-commit/ and add proper PR title and description to #4987.

[smanna12]

Can you add test?

Tests have been already added. Sorry I missed that.

[s-kanaev]

@premanandrao , a friendly ping

[elizabethandrews]

I'll check with @elizabethandrews what sort of test should be added here

There is a test in this patch for the macro. From a FE perspective, I don't think further tests are required

[premanandrao]

LGTM