[SYCL] Add MSVC predefined type characteristics for wchar VaListKind,… #284
Conversation
|
Could you please shorten the subject of the commit message. Please move details to commit message body. |
Add MSVC-specific predefined type characteristics for wchar, builtin VaListKind, and a temporary workaround for the vectorcall attribute. We'll allow vectorcall to appear on function declarations, since they occur in the Microsoft headers, but we won't allow or will ignore vectorcall on function invocations from SYCL target. The handling for vectorcall is a work in progress. Signed-off-by: Melanie Blower <[email protected]>
| @@ -74,6 +74,8 @@ class LLVM_LIBRARY_VISIBILITY SPIRTargetInfo : public TargetInfo { | |||
| // Define available target features | |||
| // These must be defined in sorted order! | |||
| NoAsmVariants = true; | |||
| if (Triple.isWindowsMSVCEnvironment() && Triple.isSYCLDeviceEnvironment()) | |||
There was a problem hiding this comment.
Is this change to SYCL device compiler?
If so, I'm not sure I understand that msvc environment is valid configuration here.
According to my understanding it can be used only for aux-triple, but target triple, which is currently can be only SPIR.
I think setting msvc environment might be interpreted by the compiler that the device compiler supports MCVC runtime libraries, which might not be provided by the OpenCL implementation.
Could clarify, please?
There was a problem hiding this comment.
When compiling with Windows system headers, for compatibility with Windows, the sizeof wchar needs to be unsigned short. @romanovvlad wants this to be restricted to sycl device so that it doesn't interfere with OpenCL, @v-klochkov requested the msvc restricton
There was a problem hiding this comment.
PS The aux-triple isn't completely implemented yet, if possible i'll change the if- to something more acceptable when the aux-triple changes are fixed/incorporated
There was a problem hiding this comment.
Melanie, when you say that aux triple changes not incorporated yet, what do you mean?
// (1). Generate LLVM IR for SPIR64 target
"C:\Users\vklochko\ws\gs_winfix2_public\build\bin\clang.EXE" "-cc1" "-triple" "spir64-unknown-windows-sycldevice" "-std=c++14" "-fms-extensions" "-fsycl-is-device" "-aux-triple" "x86_64-pc-windows-msvc" "-disable-llvm-passes" "-emit-llvm-bc" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "accessor.cpp" ... ... ... ...
Here we have triple="spir64-unknown-windows-sycldevice" and aux-triple="x86_64-pc-windows-msvc"
Is there some way to check that a) triple targets sycldevice and b) aux-triple targets msvc ?
There was a problem hiding this comment.
Currently the settings from aux-triple aren't available here, i'm preparing a separate patch to address this problem.
There was a problem hiding this comment.
Ok, understood. I didn't know about problems with aux-triple.
Then for now I suggest replacing 'isWindowsMSVCEnvironment()' (i.e. "-aux-triple" "x86_64-pc-windows-msvc") with check of Windows only from here (spir64-unknown-windows-sycldevice)
There was a problem hiding this comment.
@v-klochkov I'm sorry i don't know what you mean exactly, you want me to change SPIR.h to use "isWindows" instead of the current? And you want me to change the lit test case too?
Alternatively can i add a FIXME comment?
There was a problem hiding this comment.
@mibintc, today SYCL device compiler support compilation only for SPIR target (or one of its sub-targets) and only for sycldevice environment (i.e. spir/spir64---sycldevice).
This PR enables unsupported test case, so it's not clear why it's implemented this way.
To detect compilation for Windows, we should check "OS" triple component.
isWindowsMSVCEnvironment check doesn't make sense for SYCL device compilation.
|
I have created updates to this PR, they are available for code review in #325 |
… temporary workaround vectorcall problem
Signed-off-by: Melanie Blower [email protected]