Thanks for the minimal patch!
LGTM - see inline for some cosmetics.
Nit: If we don't use the current/correct formatting for a function name (roundingModeMayBe), we're never going to get out of the formatting mess we're in.
Bonus points for fixing all of the functions here (as an NFC preliminary patch). :)
This is complicated logic, so it would be good to put a code comment/examples on it. Something like:
// With strict/constrained FP, we have these possible edge cases that do not simplify to Op0: // fadd SNaN, -0.0 --> QNaN // fadd +0.0, -0.0 --> -0.0 (with round toward negative)
Independent of this patch, but I wonder if it is too late to unify the vocabulary on the metadata and enum names.
Does this apply to type names that appear in the name of functions? For example, RoundingModeToStr() takes a RoundingMode argument. That was part of why I went with RoundingModeMayBe(). (The rest of the reason was to be consistent with the rest of this file.)
Yeah, I looked into that. I agree that FPEnv.h is the right place.
But when I tried it I found that FastMathFlags is in Operator.h, and that seems like a heavy file to include in FPEnv.h. In the near future I'm planning on trying to move the FastMathFlags out into a standalone header that I can pull into Operator.h and FPEnv.h, and that will allow me to move canIgnoreSNaN() into FPEnv.h.
Good question. I see that llvm::RoundingMode mirrors the names in IEEE-754. So if we change anything it should be the strings accepted by the constrained intrinsics. I guess that's something someone could do especially since the constrained intrinsics are still in the experimental namespace? With upgrade support of course.
I didn't notice that quirk of naming...
Oh, @sepavloff, are you good with me adding, in a different commit, the word "convert" to the beginning of these four conversion functions?