aaron.ballman added inline comments.
================
Comment at: clang/docs/LanguageExtensions.rst:3399
+by the pragma behaves as though the command-line option
+``-ffp-eval-method=source`` is enabled, returning floating point evaluation
+method to the default setting.
----------------
================
Comment at: clang/docs/UsersManual.rst:1486
+ Valid values are: ``source``, ``double``, and ``extended``.
+ The default value is target specific, typically ``source``. Details:
+
----------------
================
Comment at: clang/include/clang/Basic/LangOptions.h:226
+ /// Possible float expression evaluation method choies.
+ enum FPEvalMethodKind {
----------------
================
Comment at: clang/include/clang/Lex/Preprocessor.h:181
IdentifierInfo *Ident__is_target_environment; // __is_target_environment
+ IdentifierInfo *Ident__FLT_EVAL_METHOD__; // __FLT_EVAL_METHOD__
----------------
Otherwise, when `LexExpandSpecialBuiltins` is `false`, I think this winds up
being an uninitialized pointer value.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2803
+ // Validate and pass through -ffp-float-precision option.
+ case options::OPT_ffp_eval_method_EQ: {
----------------
================
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1743
});
- } else if (II == Ident__has_include ||
- II == Ident__has_include_next) {
+ } else if (II == Ident__has_include || II == Ident__has_include_next) {
// The argument to these two builtins should be a parenthesized
----------------
Unrelated formatting change?
================
Comment at: clang/lib/Sema/Sema.cpp:216
+ // Use setting from TargetInfo.
+ PP.setCurrentFltEvalMethod(static_cast<LangOptions::FPEvalMethodKind>(
+ ctxt.getTargetInfo().getFloatEvalMethod()));
----------------
I don't think this cast is needed, is it? (It might make sense for the
`getFloatEvalMethod()` return type to be `int` rather than `unisnged` though,
given that we support negative values for implementation-defined semantics?)
================
Comment at: clang/lib/Sema/Sema.cpp:220
+ // Set initial value of __FLT_EVAL_METHOD__ from the command line.
+ PP.setCurrentFltEvalMethod(getLangOpts().getFPEvalMethod());
}
----------------
Heh, so we have:
`setCurrentFltEvalMethod`
`getFloatEvalMethod`
`getFPEvalMethod`
I think we should pick a consistent spelling for `float evaluation method` and
go with it. I don't know that work needs to happen as part of this patch (it
can be a follow-up), but I think it'd help with readability.
================
Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:240
+
+float mySub(float x, float y) {
+ // CHECK: define {{.*}}float {{.*}}mySub{{.*}}
----------------
Can we also get some tests that show how this behaves with the less common
floating-point types like float16, bfloat16, float128, half?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93769/new/
https://reviews.llvm.org/D93769
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
