[PATCH] D39931: AMDGPU: Don't try to enable fp64 denormals on <SI

Jan Vesely via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 12 21:05:59 PST 2017


jvesely added a comment.

In https://reviews.llvm.org/D39931#922904, @arsenm wrote:

> Fp64 denormals are always supported
>
> In https://reviews.llvm.org/D39931#922846, @jvesely wrote:
>
> > In https://reviews.llvm.org/D39931#922825, @arsenm wrote:
> >
> > > FP64 support without denormals doesn't make sense. They are mandatory in the OpenCL spec
> >
> >
> > so what? this has clearly been disabled before, and it stays disabled after this change, just with less damage done in the process..
> >
> > enabling fp64 denormals can be considered when fp64 support is added to eg/ni targets.
>
>
> The Evergreen manual lists double_denorm_flush_input and double_denorm_force_underflow_to_zero modes, so that implies they are supported.


I'll double check if it matches it specs when fp64 gets added, afaik it's not consistent for all instructions.

> I don't care that much because the f64 support is missing, but I'm not sure what problem you are trying to solve here

the problem is that  fp64-fp16-denormals feature depends on fp64 feature, thus passing "+fp64-fp16-denormals" string enables fp64 on all ASICs, including those (like almost all EG) that don't support fp64. THus I can't use 'if (hasFP64())' to in isel.
This needs to work for FMA (not available without fp64, the instruction executes fine but returns 0).

The alternative is to drop fp64-fp16-denormlas dependency on fp64 and change the has*() function to check for both.


Repository:
  rL LLVM

https://reviews.llvm.org/D39931





More information about the llvm-commits mailing list