Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LangRef] Adjust the documentation of some fast-math flags. #99557

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

jcranmer-intel
Copy link
Contributor

The first change is the clarification of rewrite-based semantics, and the fact that when doing the rewrite, all of the instructions involved need to have the rewrite. This is not a change in semantics: there is wide agreement that this behavior is true for most flags. But it is necessary to clarify this, and also clarify that there is a fundamental difference between a flag like nnan and a flag like contract. Note that several InstCombine transforms do not correctly check this behavior at the moment.

The second change is a specific clarification of the rewrites performed by arcp. These rewrites capture what is necessary to enable the transformations that currently require just arcp, none of which are using the flag incorrectly right now.

The first change is the clarification of rewrite-based semantics, and
the fact that when doing the rewrite, all of the instructions involved
need to have the rewrite. This is not a change in semantics: there is
wide agreement that this behavior is true for most flags. But it is
necessary to clarify this, and also clarify that there is a fundamental
difference between a flag like `nnan` and a flag like `contract`. Note
that several InstCombine transforms do not correctly check this
behavior at the moment.

The second change is a specific clarification of the rewrites performed
by arcp. These rewrites capture what is necessary to enable the
transformations that currently require just arcp, none of which are
using the flag incorrectly right now.
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-llvm-ir

Author: Joshua Cranmer (jcranmer-intel)

Changes

The first change is the clarification of rewrite-based semantics, and the fact that when doing the rewrite, all of the instructions involved need to have the rewrite. This is not a change in semantics: there is wide agreement that this behavior is true for most flags. But it is necessary to clarify this, and also clarify that there is a fundamental difference between a flag like nnan and a flag like contract. Note that several InstCombine transforms do not correctly check this behavior at the moment.

The second change is a specific clarification of the rewrites performed by arcp. These rewrites capture what is necessary to enable the transformations that currently require just arcp, none of which are using the flag incorrectly right now.


Full diff: https://github.com/llvm/llvm-project/pull/99557.diff

1 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+48-5)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index cd86156ec816f..af852782741c7 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -3669,6 +3669,10 @@ LLVM IR floating-point operations (:ref:`fneg <i_fneg>`, :ref:`fadd <i_fadd>`,
 may use the following flags to enable otherwise unsafe
 floating-point transformations.
 
+``fast``
+   This flag is a shorthand for specifying all fast-math flags at once, and
+   imparts no additional semantics from using all of them.
+
 ``nnan``
    No NaNs - Allow optimizations to assume the arguments and result are not
    NaN. If an argument is a nan, or the result would be a nan, it produces
@@ -3684,9 +3688,51 @@ floating-point transformations.
    argument or zero result as insignificant. This does not imply that -0.0
    is poison and/or guaranteed to not exist in the operation.
 
+Rewrite-based flags
+^^^^^^^^^^^^^^^^^^^
+
+The following flags have rewrite-based semantics. These flags allow expressions,
+potentially containing multiple non-consecutive instructions, to be rewritten
+into alternative instructions. When multiple instructions are involved in an
+expression, it is necessary that all of the instructions have the necessary
+rewrite-based flag present on them, and the rewritten instructions will
+generally have the intersection of the flags present on the input instruction.
+
+In the following example, the floating-point expression in the body of ``@orig``
+has ``contract`` and ``reassoc`` in common, and thus if it is rewritten into the
+expression in the body of ``@target``, all of the new instructions get those two
+flags and only those flags as a result. Since the ``arcp`` is present on only
+one of the instructions in the expression, it is not present in the transformed
+expression. Furthermore, this reassociation here is only legal because both the
+instructions had the ``reassoc`` flag; if only one had it, it would not be legal
+to make the transformation.
+
+.. code-block:: llvm
+
+      define double @orig(double %a, double %b, double %c) {
+        %t1 = fmul contract reassoc double %a, %b
+        %val = fmul contract reassoc arcp double %t1, %c
+        ret double %val
+      }
+
+      define double @target(double %a, double %b, double %c) {
+        %t1 = fmul contract reassoc double %b, %c
+        %val = fmul contract reassoc double %a, %t1
+        ret double %val
+      }
+
+These rules do not apply to the other fast-math flags. Whether or not a flag
+like ``nnan`` is present on any or all of the rewritten instructions is based
+on whether or not it is possible for said instruction to have a NaN input or
+output, given the original flags.
+
 ``arcp``
-   Allow Reciprocal - Allow optimizations to use the reciprocal of an
-   argument rather than perform division.
+   Allows division to be treated as a multiplication by a reciprocal.
+   Specifically, this permits ``a / b`` to be considered equivalent to
+   ``a * (1.0 / b)`` (which may subsequently be susceptible to code motion),
+   and it also permits ``a / (b / c)`` to be considered equivalent to
+   ``a * (c / b)``. Both of these rewrites can be applied in either direction:
+   ``a * (c / b)`` can be rewritten into ``a / (b / c)``.
 
 ``contract``
    Allow floating-point contraction (e.g. fusing a multiply followed by an
@@ -3705,9 +3751,6 @@ floating-point transformations.
    Allow reassociation transformations for floating-point instructions.
    This may dramatically change results in floating-point.
 
-``fast``
-   This flag implies all of the others.
-
 .. _uselistorder:
 
 Use-list Order Directives

@jcranmer-intel
Copy link
Contributor Author

This change also subsumes the #89442 change.

@arsenm arsenm added the floating-point Floating-point math label Jul 19, 2024
@jcranmer-intel jcranmer-intel merged commit 858bea8 into llvm:main Aug 2, 2024
11 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 2, 2024

LLVM Buildbot has detected a new failure on builder premerge-monolithic-windows running on premerge-windows-1 while building llvm at step 8 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/1608

Here is the relevant piece of the build log for the reference:

Step 8 (test-build-unified-tree-check-all) failure: test (failure)
...
[37/125] Linking CXX executable tools\polly\unittests\Flatten\FlattenTests.exe
[38/125] Linking CXX executable tools\flang\unittests\Optimizer\FlangOptimizerTests.exe
[39/125] Linking CXX executable tools\clang\unittests\Sema\SemaTests.exe
[40/125] Linking CXX executable tools\clang\unittests\Rewrite\RewriteTests.exe
[41/125] Linking CXX executable tools\clang\unittests\AST\Interp\InterpTests.exe
[42/125] Linking CXX executable tools\clang\unittests\CrossTU\CrossTUTests.exe
[43/125] Linking CXX executable tools\clang\unittests\ASTMatchers\Dynamic\DynamicASTMatchersTests.exe
[44/125] Linking CXX executable tools\clang\unittests\Analysis\FlowSensitive\ClangAnalysisFlowSensitiveTests.exe
[45/125] Linking CXX executable tools\clang\tools\extra\unittests\clang-tidy\ClangTidyTests.exe
[46/125] Preparing lit tests
FAILED: utils/lit/CMakeFiles/prepare-check-lit 
cmd.exe /C "cd /D C:\ws\buildbot\premerge-monolithic-windows\build\utils\lit && C:\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe -E remove_directory C:/ws/buildbot/premerge-monolithic-windows/build/utils/lit/tests && C:\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe -E copy_directory C:/ws/buildbot/premerge-monolithic-windows/llvm-project/llvm/utils/lit/tests C:/ws/buildbot/premerge-monolithic-windows/build/utils/lit/tests && C:\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe -E copy C:/ws/buildbot/premerge-monolithic-windows/build/utils/lit/lit.site.cfg C:/ws/buildbot/premerge-monolithic-windows/build/utils/lit/tests"
Error removing directory "C:/ws/buildbot/premerge-monolithic-windows/build/utils/lit/tests".
[47/125] Linking CXX executable tools\clang\unittests\Tooling\Syntax\SyntaxTests.exe
[48/125] Linking CXX executable tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe
[49/125] Linking CXX executable tools\polly\unittests\Support\ISLToolsTests.exe
[50/125] Linking CXX executable tools\clang\unittests\AST\ASTTests.exe
[51/125] Linking CXX executable unittests\CodeGenData\CodeGenDataTests.exe
[52/125] Linking CXX executable unittests\Bitcode\BitcodeTests.exe
[53/125] Linking CXX executable tools\polly\unittests\DeLICM\DeLICMTests.exe
[54/125] Linking CXX executable unittests\DebugInfo\BTF\DebugInfoBTFTests.exe
[55/125] Linking CXX executable tools\clang\unittests\Index\IndexTests.exe
[56/125] Linking CXX executable tools\polly\unittests\ScopPassManager\ScopPassManagerTests.exe
[57/125] Linking CXX executable tools\clang\unittests\ASTMatchers\ASTMatchersTests.exe
[58/125] Linking CXX executable tools\polly\unittests\ScheduleOptimizer\ScheduleOptimizerTests.exe
[59/125] Linking CXX executable tools\clang\unittests\Rename\ClangRenameTests.exe
[60/125] Linking CXX executable unittests\Analysis\AnalysisTests.exe
[61/125] Linking CXX executable tools\clang\unittests\Serialization\SerializationTests.exe
[62/125] Linking CXX executable tools\clang\unittests\Support\ClangSupportTests.exe
[63/125] Linking CXX executable tools\clang\unittests\CodeGen\ClangCodeGenTests.exe
[64/125] Linking CXX executable tools\mlir\unittests\Target\LLVM\MLIRTargetLLVMTests.exe
[65/125] Linking CXX executable tools\clang\unittests\Driver\ClangDriverTests.exe
[66/125] Linking CXX executable tools\clang\unittests\Frontend\FrontendTests.exe
[67/125] Linking CXX executable bin\mlir-capi-pass-test.exe
[68/125] Linking CXX executable bin\mlir-capi-ir-test.exe
[69/125] Linking CXX executable bin\mlir-capi-execution-engine-test.exe
[70/125] Linking CXX executable tools\lld\unittests\AsLibELF\LLDAsLibELFTests.exe
[71/125] Linking CXX executable tools\lld\unittests\AsLibAll\LLDAsLibAllTests.exe
[72/125] Linking CXX executable tools\clang\tools\extra\clangd\unittests\ClangdTests.exe
[73/125] Linking CXX executable unittests\CodeGen\CodeGenTests.exe
[74/125] Linking CXX executable unittests\CodeGen\GlobalISel\GlobalISelTests.exe
[75/125] Linking CXX executable tools\clang\unittests\Interpreter\ClangReplInterpreterTests.exe
[76/125] Linking CXX executable tools\flang\unittests\Frontend\FlangFrontendTests.exe
[77/125] Linking CXX executable tools\clang\unittests\Tooling\ToolingTests.exe
ninja: build stopped: subcommand failed.

banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
The first change is the clarification of rewrite-based semantics, and
the fact that when doing the rewrite, all of the instructions involved
need to have the rewrite. This is not a change in semantics: there is
wide agreement that this behavior is true for most flags. But it is
necessary to clarify this, and also clarify that there is a fundamental
difference between a flag like `nnan` and a flag like `contract`. Note
that several InstCombine transforms do not correctly check this behavior
at the moment.

The second change is a specific clarification of the rewrites performed
by arcp. These rewrites capture what is necessary to enable the
transformations that currently require just arcp, none of which are
using the flag incorrectly right now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
floating-point Floating-point math llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants