[PATCH] D69598: Work on cleaning up denormal mode handling

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added a comment.

In D69598#1739655 <https://reviews.llvm.org/D69598#1739655>, @andrew.w.kaylor wrote:

> I'm unclear as to the expectations surrounding this option. I suppose this is somewhat beyond the scope of the current changes, but I'm confused by clang's current behavior with regard to denormals.


Yes, the current usage is underspecified and broken by default. I complained about this in this post: http://lists.llvm.org/pipermail/llvm-dev/2019-November/136449.html
The difference between whether input denormals are implicitly flushed and whether denormal results are flushed to zero does matter in the current codegen use.

> The -fdenromal-fp-math option causes a function attribute to be set indicating the desired flushing behavior, and I guess in some cases that has an effect on instruction selection, but it seems to be orthogonal to whether or not we're actually setting the processor controls to do flushing (at least for most targets). I really only know what happens in the x86 case, and I don't know if this behavior is consistent across architectures, but in the x86 case setting or not setting the processor  control flags depends on the fast math flags and whether or not we find crtfastmath.o when we link.

The current user needs to know if it can safely ignore a denormal input to avoid miscompiling sqrt. For AMDGPU this changes some lowering and instructions that are safely selectable. I would also like to be able possibly use this for constant folding llvm.canonicalize, although I'm unsure if we need a "may flush" or "must flush" distinction.

> This leads me to my other point of confusion. Setting the "denormal-fp-math" option on a per-function basis seems wrong for targets that have a global FP control.

That's really going to be all targets. For AMDGPU we can directly set the FP mode for the kernels/entry points from this attribute, but not an arbitrary callable function.  I do think the floating point environment bits should be a considered a property of the calling convention, with attributes that override them. A function which calls a function with a different mode would be responsible for switching the mode before the call. This would require people actually caring about getting this right to really implement



================
Comment at: clang/include/clang/Basic/CodeGenOptions.h:167
   /// The floating-point denormal mode to use.
-  std::string FPDenormalMode;
+  llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::Invalid;
 
----------------
andrew.w.kaylor wrote:
> Why is "Invalid" the default here? If you don't use the "fdenormal-fp-math" option, shouldn't you get IEEE?
Because the current users are broken, and this minimizes changes in the cleanup patches. The follow up patches fix this and switch the default


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

https://reviews.llvm.org/D69598