[PATCH] D139481: NVPTX: Cleanup check for denormal mode

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp:374
+        Action.IsHalfTy ? APFloat::IEEEhalf() : APFloat::IEEEsingle());
+    bool FtzEnabled = Mode.Output == DenormalMode::PreserveSign;
 
----------------
arsenm wrote:
> tra wrote:
> > arsenm wrote:
> > > Looks broken for double to me but I don't know what the options are
> > 
> > Indeed. I do not understand why getDenormalMode appears to single out fp32 as a special case.
> > 
> > 
> Because turn on f32 flush is the option the languages actually expose. denormal-fp-math-f32 acts as an override to denormal-fp-math
OK, then the code is (and has been) broken for 'double'. I do not think we want to override denormals mode for doubles using `denormal-fp-math-f32`.
On the other hand, it's no more broken than it was before, so we can probably live with that until we figure out how to fix this.

https://llvm.org/docs/LangRef.html says:
>" denormal-fp-math-f32" [...] Not all targets support separately setting the denormal mode per type, and no attempt is made to diagnose unsupported uses. Currently this attribute is respected by the AMDGPU and NVPTX backends.

On a side note, I wonder if the denormals handling mode may need to grow a similar override for fp16, which also has .ftz instruction variants.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139481/new/

https://reviews.llvm.org/D139481