This is an archive of the discontinued LLVM Phabricator instance.

[clang][docs] Correct floating point option explanations
ClosedPublic

Authored by kawashima-fj on Nov 16 2022, 4:05 AM.

Details

Summary

Explanations for options of floating point are updated to match the RenderFloatingPointOptions function in clang/lib/Driver/ToolChains/Clang.cpp.

Missing explanations are also added.

Diff Detail

Event Timeline

kawashima-fj created this revision.Nov 16 2022, 4:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2022, 4:05 AM
kawashima-fj requested review of this revision.Nov 16 2022, 4:05 AM

I don't know it is inteded that -fno-fast-math does not imply -ftrapping-math and -frounding-math.
-ffast-math and -fno-fast-math is not symmetric in RenderFloatingPointOptions.

It would probably be better to make these changes with the appropriate code changes you are making for each semantic mode. And may be keep this patch only for the other changes you are proposing.

I don't know it is inteded that -fno-fast-math does not imply -ftrapping-math and -frounding-math.
-ffast-math and -fno-fast-math is not symmetric in RenderFloatingPointOptions.

I would think that there should be symmetry between ffast-math and fno-fast-math. Unless there is a good reason for it, in which case a comment should be added? @andrew.w.kaylor WDYT?

It would probably be better to make these changes with the appropriate code changes you are making for each semantic mode. And may be keep this patch only for the other changes you are proposing.

Sorry, I don't understand what you mean.
I didn't/won't change code (except a bug fix of D138109) or propose changing code.
I just want the document to be consistent with the existing code by this patch.
Are you saying about D138109? Or other things?

(Sorry, I'm not a native English speaker. My understanding may be bad.)

It would probably be better to make these changes with the appropriate code changes you are making for each semantic mode. And may be keep this patch only for the other changes you are proposing.

Sorry, I don't understand what you mean.
I didn't/won't change code (except a bug fix of D138109) or propose changing code.
I just want the document to be consistent with the existing code by this patch.
Are you saying about D138109? Or other things?

(Sorry, I'm not a native English speaker. My understanding may be bad.)

@@kawashima-fj no need to apologize!
What I meant was that the changes about the semantic mode approx-func that you made here https://reviews.llvm.org/D138109 , should have included the changes that you are making in this patch about approx-func.
In general, the changes to the users' manual should be in the same patch than the code changes.

What I meant was that the changes about the semantic mode approx-func that you made here https://reviews.llvm.org/D138109 , should have included the changes that you are making in this patch about approx-func.
In general, the changes to the users' manual should be in the same patch than the code changes.

Thanks. I understand.
Regarding D138109, the current manual already has the following sentences.

  • precise Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (-ffp-contract=on). This is the default behavior.
  • strict Enables -frounding-math and -ffp-exception-behavior=strict, and disables contractions (FMA). All of the -ffast-math enablements are disabled. Enables STDC FENV_ACCESS: by default FENV_ACCESS is disabled. This option setting behaves as though #pragma STDC FENV_ACESS ON appeared at the top of the source file.

These indicates that precise and strict should disable -fapprox-func. D138109 changes the code to be consistent with the manual.
So D138109 does not need to update the manual and this patch does not update the corresponding sentences.

zahiraam added inline comments.Nov 29 2022, 5:58 AM
clang/docs/UsersManual.rst
1435

Leave a blank line before and after the bullet. Same below.

1612

-fno-signed-zeros. Same below.

1628

behave when combined.

1638

`-ffinite-math-only` implies:

kawashima-fj edited the summary of this revision. (Show Details)

Addressed to @zahiraam's comment

kawashima-fj marked 4 inline comments as done.Dec 1 2022, 6:35 AM
zahiraam accepted this revision.Dec 1 2022, 6:59 AM

Addressed to @zahiraam's comment

Looks good.

This revision is now accepted and ready to land.Dec 1 2022, 6:59 AM