rjmccall added a comment.
Just minor requests now.
================
Comment at: clang/docs/LanguageExtensions.rst:3177
+Both floating point reassociation and floating point contraction can be
+controlled with this pragma.
+``#pragma clang fp reassoc`` allows control over the reassociation
----------------
rjmccall wrote:
> Let's go ahead and word this as if arbitrary things will be controllable in
> the future. So:
>
> > Currently, the following things can be controlled by this pragma:
Thanks. Please put the first sentence in its own paragraph and end it with a
colon so that it logically leads into both of the following blocks.
================
Comment at: clang/docs/LanguageExtensions.rst:3182
+enabled for the translation unit with the ``-fassociative-math`` flag.
+The pragma can take two values: ``on`` and ``off``.
+
----------------
Do you want to add an example here?
================
Comment at: clang/docs/LanguageExtensions.rst:3192
in cases when the language standard does not make this possible (e.g. across
statements in C)
----------------
Oh, and there's a missing period here.
================
Comment at: clang/include/clang/Basic/LangOptions.h:186
+ FPM_Fast
};
----------------
mibintc wrote:
> rjmccall wrote:
> > I'm not sure I think this fusion was an improvement; the net effect was to
> > remove a few lines from this header and make a bunch of switches
> > unnecessarily non-exhaustive.
> I dropped the on/off enumeration and just using boolean, where needed, to
> show the setting, do you like this better?
Yeah, thanks.
================
Comment at: clang/include/clang/Sema/Sema.h:9624
+ /// \#pragma clang fp reassociate
+ void ActOnPragmaFPAllowReassociation(bool IsEnabled);
----------------
Please name this the same as the language feature.
================
Comment at: clang/lib/Parse/ParsePragma.cpp:2900
return;
}
PP.Lex(Tok);
----------------
Please make this a single block by making the condition something like `if
(!FlagValue || (FlagKind == TokFPAnnotValue::Reassociate && FlagValue ==
TokFPAnnotValue::Fast))`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
https://reviews.llvm.org/D78827
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
