tblah added inline comments.
================ Comment at: flang/include/flang/Frontend/LangOptions.h:29 + + // Enable the floating point pragma + FPM_On, ---------------- vzakhari wrote: > tblah wrote: > > vzakhari wrote: > > > tblah wrote: > > > > awarzynski wrote: > > > > > What are these pragmas? Perhaps you can add a test that would include > > > > > them? > > > > I copied this comment from clang. I believe the pragma is > > > > ``` > > > > #pragma clang fp contract(fast) > > > > ``` > > > > > > > > See > > > > https://clang.llvm.org/docs/LanguageExtensions.html#extensions-to-specify-floating-point-flags > > > > > > > > This patch only adds support for argument processing, so I can't test > > > > for the pragmas. > > > I do not think we should blindly copy this from clang. I believe > > > `-ffp-contract=on` is there to do the contraction complying to the > > > language standard - but Fortran standard says nothing about contraction. > > > I am also not aware about a Fortran compiler that supports directives > > > related to contraction, so `fast-honor-pragmas` does not seem to be > > > applicable as well. Basically, we end up with just `off` and `fast`. > > > > > > Now, it may be reasonable to support the same `-ffp-contract=` arguments > > > so that users can apply the same options sets for C/C++ and Fortran > > > compilations. If we want to do this, we need to map `on` and > > > `fast-honor-pragmas` to something, e.g. `fast`. A driver warning (not an > > > error) may be useful to make the option's behavior clear when `on` and > > > `fast-honor-pragmas` are passed. > > From the clang help text: > > ``` > > Form fused FP ops (e.g. FMAs): > > fast (fuses across statements disregarding pragmas) > > | on (only fuses in the same statement unless dictated by pragmas) > > | off (never fuses) > > Default is 'on' > > ``` > > > > So if we removed `on` and set the default to `off` we would no longer fuse > > within the same statement by default. > > > > Classic-flang seems to support `on`, `off` and `fast`: > > https://github.com/flang-compiler/flang/blob/master/test/llvm_ir_correct/fma.f90 > Not talking about defaults just yet, I think Flang cannot currently support > the `on` mode as documented. > > I do not have the latest classic flang readily availalbe, but I am curious > what it will generate for this example: > ``` > function fn(x,y,z) > real :: x,y,z > fn = x * y > fn = fn + z > end function > ``` > > With a very old classic flang I get just `fast` math flags on the multiple > and add instructions, which is obviously not what `on` is supposed to do: > ``` > $ flang -target aarch64-linux-gnu -O1 -c -S -emit-llvm -ffp-contract=on > fma.f90 > $ cat fma.ll > %4 = fmul fast float %3, %1, !dbg !10 > %5 = bitcast i64* %z to float*, !dbg !11 > %6 = load float, float* %5, align 4, !dbg !11 > %7 = fadd fast float %6, %4, !dbg !11 > ``` > > Maybe the latest classic flang does support it properly, e.g. it only > contracts operations from the same statement. But I do not see a way to > support this in Flang right now, so documenting the `on` mode as it is in > clang seems confusing. > > We can still support `on` in the Flang option, but I think we need to issue a > warning saying that it defaults to something else, e.g. to `fast`. If > mapping `on` to `fast` is not appropriate to some users, then they will have > to explicitly specify `-ffp-contract=off` for Fortran compilations in their > build system. > > I am also curious what `fuses in the same statement` means for Fortran. For > example: > ``` > x1 = DOT_PRODUCT(x2, x3)+x4*x5+x6 > ``` > > If Fortran processor decides to implement `DOT_PRODUCT` as inline > multiply+add loop, does `-ffp-contract=on` apply to them or it only applies > to `x4*x5+x6`? Thanks for your feedback. I've updated my patch. Now flang only supports `off` and `fast`. The other two map to `fast` and we default to `off`. gfortran seems to default to `fast`: ``` -ffp-contract=style -ffp-contract=off disables floating-point expression contraction. -ffp-contract=fast enables floating-point expression contraction such as forming of fused multiply-add operations if the target has native support for them. -ffp-contract=on enables floating-point expression contraction if allowed by the language standard. This is currently not implemented and treated equal to -ffp-contract=off. The default is -ffp-contract=fast. ``` https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html ================ Comment at: flang/test/Driver/driver-help-hidden.f90:34 ! CHECK-NEXT: Use <value> as character line width in fixed mode +! CHECK-NEXT: -ffp-contract=<value> Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses in the same statement unless dictated by pragmas) | off (never fuses) | fast-honor-pragmas (fuses across statements unless diectated by pragmas). Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on' otherwise. ! CHECK-NEXT: -ffree-form Process source files in free form ---------------- awarzynski wrote: > tblah wrote: > > vzakhari wrote: > > > Is it easy to emit a different help message for Flang to say that there > > > are only two modes for Fortran? > > @awarzynski tells me there is no way to do this short of having separate > > `Options.td` for flang and clang. Once we have settled on which arguments > > to support, I will update the shared help text to mention flang. > Both `clang -help` and `flang-new -help` must be 100% correct. As this help > text is not valid in the case of LLVM Flang, it needs to be updated > accordingly. > > As @tblah points out, there's no straightforward mechanism for having a > custom help texts for `clang` and `flang-new` in Clang's driver library ATM. > But I think that this can be achieved even without creating a separate > "Options.td" file. One would have to define a new tablegen record in > "Options.td". That would be a separate patch though, probably accompanied by > an RFC. > > There's a different solution too. Note that currently the definition uses the > `HelpText` field. However, you could use [[ > https://github.com/llvm/llvm-project/blob/b1d7a95e4e4a2b57cbe02636bbe357dc48d615c5/clang/include/clang/Driver/Options.td#L81-L82 > | DocBrief ]] instead (which we used to solve a similar issue with [[ > https://github.com/llvm/llvm-project/blob/b1d7a95e4e4a2b57cbe02636bbe357dc48d615c5/clang/include/clang/Driver/Options.td#L696-L704 > | -I ]]). That's what I suggest that you do. Basically, copy the contents of > `HelpText` for `-ffp-contract` into a `DocBrief` field (we don't use this > field in Flang and it should probably be renamed as `DocBriefClang`). > `HelpText` should be replaced with something brief that applies both to Clang > and Flang. > > Btw, what's the help-text/spelling in `gfortran`? Thanks for the feedback. I have added a mention of flang to the old help, which has been moved to `DocBrief`. The new `HelpText` just mentions which options are supported by each frontend without explaining them. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
