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

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() ||
+                       FPFeatures.allowFPContractWithinStatement());
 }
----------------
mibintc wrote:
> mibintc wrote:
> > 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
> @michele.scandale I posted a patch for 'contract' here, https://reviews.llvm.org/D79903 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841