[PATCH] D31167: Use FPContractModeKind universally

[Adam Nemet via Phabricator via cfe-commits]

anemet added inline comments.


================
Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220
+/// \brief FP_CONTRACT mode (on/off/fast).
+ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type")
 LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
----------------
yaxunl wrote:
> anemet wrote:
> > yaxunl wrote:
> > > This change seemed to cause some performance degradations in some OpenCL applications.
> > > 
> > > This option used to be on by default. Now it is off by default.
> > > 
> > > There are applications do separated compile/link/codegen stages. In the codegen stage, clang is invoked with .bc as input, therefore this option is off by default, whereas it was on by default before this change.
> > > 
> > > Is there any reason not to keep the original behavior?
> > > 
> > > Thanks.
> > > This change seemed to cause some performance degradations in some OpenCL applications.
> > > 
> > > This option used to be on by default. Now it is off by default.
> > 
> > It's always been off.  It was set to fast for CUDA which should still be the case.  See the changes further down on the patch.
> > 
> > > 
> > > There are applications do separated compile/link/codegen stages. In the codegen stage, clang is invoked with .bc as input, therefore this option is off by default, whereas it was on by default before this change.
> > > 
> > > Is there any reason not to keep the original behavior?
> > 
> > Sorry but I am not sure what changed, can you elaborate on what you're doing and how things used to work for you?
> Before your change, -ffp-contract was a codegen option defined as
> 
> 
> ```
> ENUM_CODEGENOPT(FPContractMode, FPContractModeKind, 2, FPC_On)
> ```
> 
> therefore the default value as on. After your change, it becomes off by default.
No. -ffp-contract=on was a FE option and -ffp-contract=fast was a backend option.  I still don't understand your use-case.  Can you include a small testcase how this used to work before?


Repository:
  rL LLVM

https://reviews.llvm.org/D31167