scanon added a comment.
TS18661-5 is quite vague on what the intended semantics for the pragma are.
These pragmas are intended to be bindings of clause 10.4 of IEEE 754, which is
also pretty wishy-washy on the whole, but it's worth noting that clause 10 is
titled *expression evaluation*
mibintc added a comment.
The ISO C proposal is here
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2407.pdf but the details are
in the IEEE standards documents.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
rjmccall added a comment.
That's actually really interesting. Is there a paper describing the desired
model in more detail?
I think the natural interpretation of this pragma is to say that the specific
operations written within the pragma are considered to be associative and are
therefore
mibintc added a comment.
BTW there is a proposal
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2421.pdf at the ISO C meeting
to support some new floating point pragmas including
#pragma STDC FENV_ALLOW_ASSOCIATIVE_LAW on-off-switch
The committee wants to see an implementation(s) to ensure
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc355bec749e9: Add support for #pragma clang fp
reassociate(on|off) (authored by mibintc).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
rjmccall accepted this revision.
rjmccall added a comment.
LGTM, too. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
https://reviews.llvm.org/D78827
___
cfe-commits mailing list
scanon accepted this revision.
scanon added a comment.
This revision is now accepted and ready to land.
My concerns have been addressed. Thanks for bearing with me, Melanie!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
scanon added a comment.
(Please get one additional sign off before committing; I'm mainly signing off
on the numerics model aspect).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78827/new/
https://reviews.llvm.org/D78827
mibintc updated this revision to Diff 262217.
mibintc retitled this revision from "Add support for #pragma clang fp
reassociate(fast|off) -- floating point control of associative math
transformations" to "Add support for #pragma clang fp reassociate(on|off) --
floating point control of
rjmccall added a comment.
That makes sense, thanks. I think I'm comfortable using on/off for this, but
it's definitely good to stop and consider, and you're absolutely right that it
needs to be documented.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
scanon added a comment.
> I don't think the C standard is likely to ever bless reassociative FP math
> with an expression-local restriction. Steve, do you actually think that would
> be a useful optimization mode?
I think it's pretty unlikely that C would do this, as well. It is plausibly a
rjmccall added a comment.
The reason we call FP contraction "fast" instead of "on" when it's
cross-statement is because "on" is supposed to be consistent with the C
language rules, whereas "fast" is stronger. There's no analogous situation
with reassociation; I don't think the C standard is
mibintc added a comment.
I checked with FPGA folks and confirm what @scanon says is correct, the reassoc
fast math flag enables reassociation across multiple statements, so i changed
the syntax to use 'fast' and 'off', and changed the documentation
Repository:
rG LLVM Github Monorepo
mibintc updated this revision to Diff 262108.
mibintc retitled this revision from "Add support for #pragma clang fp
reassociate(on|off) -- floating point control of associative math
transformations" to "Add support for #pragma clang fp reassociate(fast|off) --
floating point control of
mibintc updated this revision to Diff 261921.
mibintc added a comment.
I fixed the issues that @rjmccall mentioned. I don't yet have an answer for
@scanon, need to get back to you about that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
scanon added inline comments.
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``.
+
mibintc wrote:
> scanon wrote:
> > rjmccall wrote:
> >
mibintc marked an inline comment as done.
mibintc added a comment.
A reply to @scanon
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``.
+
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
scanon added inline comments.
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``.
+
rjmccall wrote:
> Do you want to add an example here?
mibintc updated this revision to Diff 261875.
mibintc retitled this revision from "Add support for #pragma clang fp
reassoc(on|off) -- floating point control of associative math transformations"
to "Add support for #pragma clang fp reassociate(on|off) -- floating point
control of associative
mibintc marked 4 inline comments as done.
mibintc added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:186
+FPM_Fast
};
rjmccall wrote:
> I'm not sure I think this fusion was an improvement; the net effect was to
> remove a few
21 matches
Mail list logo
