[SYCL] Hide inline definitions of stdio functions

[hvdijk]

MSVC's inline definitions of stdio functions such as printf() are only meant for host code; device code cannot call them. Device code can, however, call ext::oneapi::experimental::printf() which depending on the target is implemented as a call to the global printf(). The availability of a definition of the global host printf() makes it more difficult to handle in device code, so hide it.

[hvdijk]

This fixes at least one SYCL E2E test (sycl/test-e2e/DeviceLib/built-ins/printf.cpp) when running on NativeCPU and presumably makes things easier for other SYCL targets as well.

[Fznamznon]

I'm not aware of implications of always enabling this macro, @AaronBallman can you maybe weigh in?

[Fznamznon]

That comment can break a brain, could you please explain the whole situation here instead?
Also, should that only happen for native cpu?

[Fznamznon]

Should we also consider enabling this macro in SYCL headers instead? It is our library implementation can't deal with this, not any SYCL.

[hvdijk]

That comment can break a brain, could you please explain the whole situation here instead?

This relates to

llvm/clang/lib/Frontend/InitPreprocessor.cpp

Lines 1630 to 1639 in 6a8df7d

	// FIXME: This will create multiple definitions for most of the predefined
	// macros. This is not the right way to handle this.
	if ((LangOpts.CUDA || LangOpts.OpenMPIsTargetDevice ||
	LangOpts.SYCLIsDevice) &&
	PP.getAuxTargetInfo())
	InitializePredefinedMacros(*PP.getAuxTargetInfo(), LangOpts, FEOpts,
	PP.getPreprocessorOpts(), Builder);
	InitializePredefinedMacros(PP.getTargetInfo(), LangOpts, FEOpts,
	PP.getPreprocessorOpts(), Builder);

, the way InitializePredefinedMacros called is not exactly ideal. It gets called twice, once for the host, once for the device, and the macros from both are used.

Also, should that only happen for native cpu?

This can simplify handling for all SYCL targets, not just NativeCPU.

Should we also consider enabling this macro in SYCL headers instead? It is our library implementation can't deal with this, not any SYCL.

We cannot enable this macro in SYCL headers since the MSVC system headers may be included before any SYCL header is included, so the macro would be defined too late.

Do we support any SYCL library other than our own?

[hvdijk]

I'm not aware of implications of always enabling this macro

For the avoidance of confusion, this is inside a if (LangOpts.SYCLIsDevice) { block, I am not suggesting always defining this macro.

[AaronBallman]

I'm not aware of implications of always enabling this macro, @AaronBallman can you maybe weigh in?

_NO_CRT_STDIO_INLINE controls whether there will be inline definitions of CRT IO functions in <conio.h>. When it's defined, the declarations are provided but with no definition. So I think this change is fine because it's guarded by SyclIsDevice.