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