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

Artem Belevich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 6 16:16:24 PST 2022


tra added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp:149-151
     // Denormal handling is guarded by different attributes depending on the
     // type (denormal-fp-math vs denormal-fp-math-f32), take note of halfs.
     bool IsHalfTy = false;
----------------
Huh. I wonder why are we using isHalfTy for figuring out denormal mode.

Considering that it seems to be FP32 that is the special case with an attribute of its own, it should've been `IsSingleTy`. Knowing whether it's fp16 or not only allows us to get correct mode for fp16, but not for the other types.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp:374
+        Action.IsHalfTy ? APFloat::IEEEhalf() : APFloat::IEEEsingle());
+    bool FtzEnabled = Mode.Output == DenormalMode::PreserveSign;
 
----------------
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.




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

https://reviews.llvm.org/D139481



More information about the llvm-commits mailing list