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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
