[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

Tue Aug 27 11:34:39 PDT 2019

rjmccall added inline comments.

================
Comment at: clang/docs/UsersManual.rst:1305
+   and ``noexcept``. Note that -fp-model=[no]except can be combined with the
+   other three settings for this option. Details:
+
----------------
mibintc wrote:
> rjmccall wrote:
> > Combined how?  With a comma?
> > 
> > This option seems to have two independent dimensions.  Is that necessary for command-line compatibility with ICC, or can we separate it into two options?
> > 
> > The documentation should mention the default behavior along both dimensions.  Is it possible to override a prior instance of this option to get this default behavior back?
> > 
> > You mention that this `-fp-model=fast` is equivalent to `-ffast-math`.  How does this option interact with that one if both are given on a command line?
> > 
> > Please put option text in backticks wherever it appears.
> > 
> > Most of these comments apply to `-fp-speculation` as well.
> > Combined how? With a comma?
> 
> > This option seems to have two independent dimensions. Is that necessary for command-line compatibility with ICC, or can we separate it into two options?
> Yes that's right, there are 2 dimensions.  I wrote it like this for identical compatibility with icc, and cl.exe also defines the option this way, to specify multiple values simultaneously. However I think it would be reasonable and good to split them into separate options.  I will discuss this with the folks back home.
> 
> > The documentation should mention the default behavior along both dimensions. 
> I added this info into the doc
> 
> > Is it possible to override a prior instance of this option to get this default behavior back?
> The 3 values along one dimension, precise, strict, fast if they appear multiple times in the command line, the last value will be the setting along that dimension.  Ditto with the other dimension, the rightmost occurrence of except or noexcept will be the setting. 
> 
> > You mention that this -fp-model=fast is equivalent to -ffast-math. How does this option interact with that one if both are given on a command line?
> The idea is that they are synonyms so if either or both appeared on the command line, the effect would be identical. 
> 
> I'll upload another patch with a few documentation updates and get back to you about splitting the fp-model option into multiple options.  (Longer term, there are 2 other dimensions to fp-model)
> 
> And thanks for the review
> Yes that's right, there are 2 dimensions. I wrote it like this for identical compatibility with icc, and cl.exe also defines the option this way, to specify multiple values simultaneously. However I think it would be reasonable and good to split them into separate options. I will discuss this with the folks back home.

Okay.  There's certainly some value in imitating existing compilers, but it sounds like a lot has been forced into one option, so maybe we should take the opportunity to split it up.  If we do split it, though, I think the different dimensions should have different base spellings, rather than being repeated uses of `-fp-model`.

> The 3 values along one dimension, precise, strict, fast if they appear multiple times in the command line, the last value will be the setting along that dimension.

Okay.  This wasn't clear to me from the code, since the code also has an "off" option.

> The idea is that they are synonyms so if either or both appeared on the command line, the effect would be identical.

Right, but compiler options are allowed to conflict with each other, with the general rule being that the last option "wins".  So what I'm asking is if that works correctly with this option and `-ffast-math`, so that e.g. `-ffast-math -fp-model=strict` leaves you with strict FP but `-fp-model=strict -ffast-math` leaves you with fast FP.  (That is another reason why it's best to have one aspect settled in each option: because you don't have to merge information from different uses of the option.)

At any rate, the documentation should be clear about how this interacts with `-ffast-math`.  You might even consider merging this into the documentation for `-ffast-math`, or at least revising that option's documentation.  Does `-fp-model=fast` cause `__FAST_MATH__` to be defined?

Also, strictly speaking, this should be `-ffp-model`, right?

================
Comment at: clang/docs/UsersManual.rst:1324
+   * ``strict`` Disables speculation on floating-point operations.
+   * ``safe`` Disables speculation if there is a possibility that speculation may cause a floating-point exception.
+
----------------
These are exclusive, right?  So the documentation should be `<value>`, not `<values>`.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62731/new/

https://reviews.llvm.org/D62731