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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
