[PATCH] D72841: Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via llvm-commits]

mibintc added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() ||
+                       FPFeatures.allowFPContractWithinStatement());
 }
----------------
michele.scandale wrote:
> I'm not convinced it correct to set `contract` when `allowFPContractWithinStatement` return true. Can someone clarify this?
> 
> If I compile the following example with `-ffp-contract=on`:
> ```
> float test1(float a, float b, float c) {
>   float x = a * b;
>   return x + c;
> }
> 
> float test2(float a, float b, float c) {
>   return a * b + c;
> }
> ```
> 
> Before this change the generated code was:
> ```
> define float @test1(float %a, float %b, float %c) {
>   %0 = fmul float %a, %b
>   %1 = fadd float %0, %c
>   ret float %1
> }
> 
> define float @test2(float %a, float %b, float %c) {
>   %0 = call float @llvm.fmuladd.f32(float %a, float%b, float %c)
>   ret float %0
> }
> ```
> 
> And my understanding is that the in-statement contraction is implemented by emitting the `llvm.fmuladd` call that a backend might decide to implement as `fmul + fadd` or as `fma`.
> 
> With this change the generated code is:
> ```
> define float @test1(float %a, float %b, float %c) {
>   %0 = fmul contract float %a, %b
>   %1 = fadd contract float %0, %c
>   ret float %1
> }
> 
> define float @test2(float %a, float %b, float %c) {
>   %0 = call contract float @llvm.fmuladd.f32(float %a, float%b, float %c)
>   ret float %0
> }
> ```
> and it seems to me that in `test1` (where multiple statements where explicitly used) the optimizer is now allowed to perform the contraction, violating the original program semantic where only "in-statement" contraction was allowed.
Thanks @michele.scandale i will work on a patch for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841