Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FP16 types are reported as unsupported for CUDA BE on compile time after cf6cc662 #1799

Closed
vladimirlaz opened this issue Jun 2, 2020 · 9 comments · Fixed by #1848
Closed
Assignees
Labels
cuda CUDA back-end

Comments

@vladimirlaz
Copy link
Contributor

vladimirlaz commented Jun 2, 2020

The problem was detected during LLVM pull down testing: http://ci.llvm.intel.com:8010/#/builders/37/builds/1152

It looks like a bug in diagnostics (AFAIK CUDA supports FP16) introduced by cf6cc66 and the tests passed before that patch.

@vladimirlaz vladimirlaz added the cuda CUDA back-end label Jun 2, 2020
@Fznamznon
Copy link
Contributor

@vladimirlaz, could you please provide CUDA spec link where it says that FP16 is supported?

@Fznamznon
Copy link
Contributor

I'm asking because nvptx target in clang doesn't define presence of float16 (FP16/half whatever) type whereas for exampler spir target does it

HasFloat16 = true;
. That is why the diagnostic is triggered. I don't know why nvptx target doesn't define presence of FP16 type, maybe not all GPUs support this type.

@Naghasan
Copy link
Contributor

Naghasan commented Jun 2, 2020

@vladimirlaz, could you please provide CUDA spec link where it says that FP16 is supported?

The target support FP16 since sm_53 I believe. This is a clash with the declared Nvidia's OpenCL capabilities.

I'm aware of this, I have a patch to enable it locally but it is entangled with builtins fixes.

This reminds me, any particular reason you are using _Float16 and not __fp16 ?
With the proper definition of the frontend, __fp16 lowers to half if and only if the targets supports it (i16 otherwise), but _Float16 is automatically lowered to the half type regardless of the target. (maybe a question for @bader )

@vladimirlaz
Copy link
Contributor Author

vladimirlaz commented Jun 2, 2020

BTW: by default we are using sm_30. I tried to uplift it up to sm_75 (including sm_53 - I also find this in some post, i did not find spec), but it is still reported as unsupported.

here is the link: https://docs.nvidia.com/cuda/cufft/index.html#half-precision-transforms

@vladimirlaz
Copy link
Contributor Author

vladimirlaz commented Jun 2, 2020

@Naghasan, @Fznamznon I am going to proceed with pulldown and need workaround for the problem.

@Naghasan do you have any ETA for the fix?

  • The easiest WA can be XFAIL LIT tests. But other staff like CTS are also affected and no WA there.
  • The other option is to revert patch from @Fznamznon until the fix is ready. You will need to reapply patch with the fix.

I would apply option 2 if there are no objections and fix will ready reasonably fast (to avoid problems with ongoing LLVM pull downs).

@Naghasan
Copy link
Contributor

Naghasan commented Jun 2, 2020

@vladimirlaz Enable it if the triple has sycldevice, I'll refine it based on the capability.

@vladimirlaz
Copy link
Contributor Author

I have prepared workaround (skip check for SYCL CUDA BE target tripple) 816febf
to unblock pulldown.

@vladimirlaz
Copy link
Contributor Author

After applying comment the a4f4fa9 was submitted to sycl branch.

@pvchupin
Copy link
Contributor

pvchupin commented Jun 9, 2020

@Fznamznon @AlexeySachkov, @erichkeane can you comment on __fp16 vs _Float16 please?

FreddyLeaf pushed a commit to FreddyLeaf/llvm that referenced this issue Mar 22, 2023
The target extension type for SPIR-V is essentially
target("spirv.TypeName", <image type>, <int params>).

Most of the work to support translation of these types has already happened
beforehand, so the primary step here is to enable translation work in
SPIRVWriter as well as making the SPIRVBuiltinHelpers work with target types as
well.

Constructing LLVM IR from SPIR-V using these types is not yet supported, mainly
out of uncertainty of the proper interface to let the resultant consumers
indicate that they wish to support these types.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@951a6ad
FreddyLeaf pushed a commit to FreddyLeaf/llvm that referenced this issue Mar 22, 2023
The expected representation is:
target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%,
%use%, (optional) %element_type_interpretation%)

TODO: figure out, how to deal with the switch from old API (Matrix has Layout) to new API (Layout was removed)

Depends on:
intel#1799
intel#8343

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@ee03f5f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda CUDA back-end
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants