[PATCH] D27028: Add intrinsics for constrained floating point operations

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

> I did not introduce the code to add a target-specific implicit use of the FP-environment register. I do intend to add this in a later patch, but this code works well enough without it that I believe it can be deferred.

I'm fine with doing this, but there should be FIXME comments in the code about this and/or bug reports filed (preferably both) so that we've not tempted to remove 'experimental' from the name until we do this part of it.

Regarding denormals, LLVM has support for the following modes (include/llvm/Target/TargetOptions.h):

  namespace FPDenormal {
    enum DenormalMode {
      IEEE,           // IEEE 754 denormal numbers
      PreserveSign,   // the sign of a flushed-to-zero number is preserved in
                      // the sign of 0
      PositiveZero    // denormals are flushed to positive zero
    };
  }

and so I recommend just following the same pattern here as with the rounding mode with strings like "denormals.dynamic", "denormals.ieee", "denormals.preserve_sign", and "denormals.positive_zero".



================
Comment at: docs/LangRef.rst:12046
+::
+      "LLVM_ROUND_DYNAMIC"
+      "LLVM_ROUND_TONEAREST"
----------------
To engage in some bikeshedding, I really don't like this naming convention which reminds me of macro names.  Not that we've been extremely consistent about the standardized metadata strings we already use, but this would be yet another choice; we should avoid that.

For controlling LLVM optimization passes, we use strings like this !"llvm.loop.vectorize.enable".  We also use strings like !"function_entry_count" and !"branch_weights" for profiling info. We also have strings like !"ProfileFormat" and !"TotalCount" in other places. Of these choices, I prefer the one used by our basic profiling information (i.e. !"branch_weights"), and so I'd prefer these be named:

  "round_dynamic"
  "round_downward"
  ...

and similar for the fpexcept strings. We might also borrow the dot-based hierarchy scheme from our loop optimization metadata and use:

  "round.dynamic"
  "round.downward"
   ...

and similar for the fpexcept strings. I think that I like this slightly better than just using the underscore separators.

I specifically dislike having "llvm" in the name here, as it makes it seem as though the meaning of these things is somehow LLVM-specific. It is not (they come from IEEE if nothing else). Having "llvm" in the optimization metadata makes a bit more sense because those refer to specific LLVM optimization passes.


================
Comment at: docs/LangRef.rst:12083
+the exception status flags will not be read and that floating point exceptions
+will not be unmasked.  This allows transformations to be performed that may
+change the exception semantics of the original code.  For example, FP operations
----------------
"not be unmasked" is a double negative. How about just saying will be masked? Or will not be enabled?


Repository:
  rL LLVM

https://reviews.llvm.org/D27028