[PATCH] D136080: [flang] Add -ffp-contract option processing
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa784de783af5: [flang] Add -ffp-contract option processing (authored by tblah, committed by kiranchandramohan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=off' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=off ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 471958. tblah added a comment. I've updated the patch with @awarzynski's nit and an up-to-date context. @kiranchandramohan please could you commit for me CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=off' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=off ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +//
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks for implementing this! Comment at: clang/lib/Driver/ToolChains/Flang.cpp:98-99 +} else + // Clang's "fast-honor-pragmas" option is not supported because it is + // non-standard and pragmas are not relevant to Fortran. + D.Diag(diag::err_drv_unsupported_option_argument) [nit] "... and pragmas are not relevant to Fortran." Fortran has directives rather than pragmas. So it's not quite like pragmas are not supported. It's just that there's different mechanism instead. 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 471163. tblah edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=off' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=off ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/ +//
[PATCH] D136080: [flang] Add -ffp-contract option processing
MaskRay added inline comments.
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:89
+const StringRef Val = A->getValue();
+if (Val.equals("fast") || Val.equals("off")) {
+ FPContract = Val;
We prefer `==` to `equals`
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
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added a comment.
Thanks for all the updates, Tom! I have a few more suggestions.
From the summary:
> implement these pragmas
Could you explain what pragmas you are referring to here? (i.e. Clang pragmas
for C and C++ + link)
> gfortran uses "fast" by default
For our future self, could you add a link as well?
Comment at: clang/include/clang/Driver/Options.td:1925
" | fast-honor-pragmas (fuses across statements unless diectated by
pragmas)."
- " Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on'
otherwise.">,
+ " Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, 'off' for flang,
and 'on' otherwise.">,
+ HelpText<"Form fused FP ops (e.g. FMAs)">,
I still think that we shouldn't be making references to Flang in Clang
documentation. And this `DocBrief` is only used by Clang. Also, "flang" is
problematic - what do you mean by "flang"?
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:91-98
+} else if (Val.equals("fast-honor-pragmas")) {
+ D.Diag(diag::warn_drv_unsupported_option_for_flang)
+ << Val << A->getOption().getName() << "fast";
+ FPContract = "fast";
+} else if (Val.equals("on")) {
+ D.Diag(diag::warn_drv_unsupported_option_for_flang)
+ << Val << A->getOption().getName() << "off";
Some "unsupported" options are treated as errors and some are warnings. I think
that for the sake of consistency it would be better to keep them all as errors.
Also, why not use `Val` instead of e.g. "off"?
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
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added a comment. Added Clang reviewers who touched the definition of `--fp-contract` most recently. Mostly for visibility, but lets give them at least a couple of days to take a look at the changes in Options.td. 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
vzakhari accepted this revision. vzakhari added a comment. This revision is now accepted and ready to land. Thank you! LGTM 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 470496. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=off' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=off ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/ +//
[PATCH] D136080: [flang] Add -ffp-contract option processing
vzakhari added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:94 + << Val << A->getOption().getName() << "fast"; + FPContract = "fast"; +} else I know I suggested myself mapping `on` to `fast`, but it seems it will be more reasonable to map it to `off`. Both my old classic flang and gfortran (12.2.0) do not generate FMAs under `on`. Moreover, mapping it to `off` may allow us to adapt gcc's help message "if allowed by the language standard" (maybe not in this commit). 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 470184. tblah marked 2 inline comments as not done. tblah edited the summary of this revision. tblah added a comment. Updated diff with a further reduced help text after discussions with @awarzynski CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=fast' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=fast ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:1926 + " Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, 'off' for flang, and 'on' otherwise.">, + HelpText<"Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only)">, Values<"fast,on,off,fast-honor-pragmas">; As far as users are concerned, `flang-new` is just a Fortran compiler. Once we start adding references to Clang in `flang-new --help`, we might be confusing users (i.e. where's the boundary between Clang and Flang?) and exposing implementation details (i.e. that `flang-new` is implemented using `clangDriver`). Ideally we should avoid that. Comment at: flang/include/flang/Frontend/LangOptions.h:29 + +// Enable the floating point pragma +FPM_On, tblah wrote: > 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 469556. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_f_opts.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/flang_f_opts.f90 === --- flang/test/Driver/flang_f_opts.f90 +++ flang/test/Driver/flang_f_opts.f90 @@ -1,8 +1,10 @@ ! Test for warnings generated when parsing driver options. You can use this file for relatively small tests and to avoid creating ! new test files. -! RUN: %flang -### -S -O4 %s 2>&1 | FileCheck %s +! RUN: %flang -### -S -O4 -ffp-contract=on %s 2>&1 | FileCheck %s +! CHECK: warning: the argument 'on' is not supported for option 'ffp-contract='. Mapping to 'ffp-contract=fast' ! CHECK: warning: -O4 is equivalent to -O3 ! CHECK-LABEL: "-fc1" +! CHECK: -ffp-contract=fast ! CHECK: -O3 Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor-pragmas (clang only) ! CHECK-NEXT: -ffree-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +//
[PATCH] D136080: [flang] Add -ffp-contract option processing
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 as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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,
[PATCH] D136080: [flang] Add -ffp-contract option processing
vzakhari added inline comments. Comment at: flang/include/flang/Frontend/LangOptions.h:29 + +// Enable the floating point pragma +FPM_On, 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`? 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added inline comments. Comment at: flang/test/Driver/driver-help-hidden.f90:34 ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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-formProcess source files in free form 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`? 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah added inline comments. Comment at: flang/include/flang/Frontend/LangOptions.h:29 + +// Enable the floating point pragma +FPM_On, 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 Comment at: flang/test/Driver/driver-help-hidden.f90:34 ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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-formProcess source files in free form 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. 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
vzakhari added inline comments. Comment at: flang/include/flang/Frontend/LangOptions.h:29 + +// Enable the floating point pragma +FPM_On, 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. Comment at: flang/test/Driver/driver-help-hidden.f90:34 ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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-formProcess source files in free form Is it easy to emit a different help message for Flang to say that there are only two modes for Fortran? 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 468840. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,4 @@ +! Test for handling of floating point options within the frontend driver + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= 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. ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= 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. ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Coding style:
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah marked 8 inline comments as done. tblah added a comment. >> The omission of the fast-honor-pragmas argument from the compiler driver is >> deliberate. > > Where is it omitted? This argument is only supported in the frontend driver, not the compiler driver: flang-new -ffp-contract=fast-honor-pragmas test.f90 flang-new: error: unsupported argument 'fast-honor-pragmas' to option '-ffp-contract=' flang-new -fc1 -ffp-contract=fast-honor-pragmas test.f90 warning: ffp-contract= is not currently implemented Comment at: flang/include/flang/Frontend/LangOptions.h:29 + +// Enable the floating point pragma +FPM_On, 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. Comment at: flang/test/Driver/flang_fp_opts.f90:4 + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented awarzynski wrote: > Can you test with `flang` as well? We already test that these flags are passed to the frontend driver from the compiler driver in `flang/test/Driver/frontend-forwarding.f90` 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
DavidTruby added a comment. Glad to see this flag being added to `flang-new`! As a note to self I've written some tests that should be updated once this lands that currently don't pass through the `flang-new` driver. Comment at: flang/test/Driver/driver-help.f90:108 ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= 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. ! HELP-FC1-NEXT: -ffree-formProcess source files in free form awarzynski wrote: > Why not expose this flag in `flang-new`? (as well as `flang-new -fc1`?) I think @awarzynski is right here, this is an option we want to expose to end users so I think the top-level driver should have it as well. As a general rule of thumb: if a flag is in `clang` as well as `clang -cc1` it should be in `flang` as well as `flang -fc1` In this case `-ffp-contract` is a `clang` flag as well as a `clang -cc1` flag, and so should probably be a `flang` flag as well as `flang -fc1` 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added a comment.
> The omission of the fast-honor-pragmas argument from the compiler driver is
> deliberate.
Where is it omitted?
> I suspect the CI failure on windows is unrelated to my code
I agree.
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:165
+ // Floating point related options
+ AddFloatingPointOptions(D, Args, CmdArgs);
+
From [[
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
| LLVM Coding style ]]
> Function names should be verb phrases (as they represent actions), and
> command-like function should be imperative. The name should be camel case,
> and start with a lower case letter (e.g. openFile() or isFoo()).
I know that this style is not followed here 100%. But that's what we should aim
for :)
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:85-86
+ArgStringList ) {
+ // TODO: share RenderFloatingPointOptions from ./Clang.cpp and use that
+ // instead of duplicating code here
+ StringRef FPContract;
tblah wrote:
> awarzynski wrote:
> > What's RenderFloatingPointOptions?
> It is a static function in clang/lib/Driver/ToolChains/Clang.cpp which does
> the same job as AddFloatingPointOptions, except for clang. I couldn't use it
> right away because not all of the options it it processes are supported in
> flang, but once we get there it would make sense to share code.
Ah! That [[
https://github.com/llvm/llvm-project/blob/b1d7a95e4e4a2b57cbe02636bbe357dc48d615c5/clang/lib/Driver/ToolChains/Clang.cpp#L2753-L3185
| function ]] is over 400 LOC and looks super complex. I doubt Flang will be
able to share that logic with Clang any time soon. If ever. I would actually
skip this comment. Sometimes code duplication is the right approach ;-)
Comment at: flang/include/flang/Frontend/LangOptions.h:9-11
+// This file defines the CodeGenOptions interface, which holds the
+// configuration for LLVM's middle-end and back-end. It controls LLVM's code
+// generation into assembly or machine code.
UPDATEME
Comment at: flang/include/flang/Frontend/LangOptions.h:29
+
+// Enable the floating point pragma
+FPM_On,
What are these pragmas? Perhaps you can add a test that would include them?
Comment at: flang/include/flang/Frontend/LangOptions.h:49-50
+
+/// Tracks various options which control the dialect of fortran that is
+/// accepted. Based on clang::LangOptions
+class LangOptions : public LangOptionsBase {
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:661-662
+/// Parses all floating point related arguments and populates the variables
+/// options accordingly. Returns false if new errors are generated.
+///
> populates the variables options accordingly
Perhaps this would be a bit more specific/descriptive:
"and configures the input CompilerInvocation accordingly". Ultimately, this is
about ... configuring the current compiler invocation, which is represented by
an instance of `CompilerInvocaiton`.
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:693
+
+// TODO: actually use the flag in codegen
+diags.Report(diagUnimplemented) << a->getOption().getName();
In here you only configure `CompilerInvocation`. This configuration will be
used elsewhere (i.e. where codegen happens). So, I think, as far as this method
is concerned the implementation is complete.
Comment at: flang/test/Driver/flang_fp_opts.f90:1-2
+! Test for passing of floating point options between the compiler and frontend
+! drivers.
+
No longer valid ;-)
Comment at: flang/test/Driver/flang_fp_opts.f90:4
+
+! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s
+! CHECK: ffp-contract= is not currently implemented
Can you test with `flang` as well?
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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah added a comment. Linux CI is passing. I suspect the CI failure on windows is unrelated to my code: the test failure is for clang-scan-deps and the previous version of the patch passed CI. 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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 468475. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CompilerInvocation.h flang/include/flang/Frontend/LangOptions.def flang/include/flang/Frontend/LangOptions.h flang/lib/Frontend/CMakeLists.txt flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/LangOptions.cpp flang/test/Driver/driver-help-hidden.f90 flang/test/Driver/driver-help.f90 flang/test/Driver/flang_fp_opts.f90 flang/test/Driver/frontend-forwarding.f90 Index: flang/test/Driver/frontend-forwarding.f90 === --- flang/test/Driver/frontend-forwarding.f90 +++ flang/test/Driver/frontend-forwarding.f90 @@ -8,6 +8,7 @@ ! RUN: -fdefault-real-8 \ ! RUN: -flarge-sizes \ ! RUN: -fconvert=little-endian \ +! RUN: -ffp-contract=fast \ ! RUN: -mllvm -print-before-all\ ! RUN: -P \ ! RUN: | FileCheck %s @@ -18,5 +19,6 @@ ! CHECK: "-fdefault-integer-8" ! CHECK: "-fdefault-real-8" ! CHECK: "-flarge-sizes" +! CHECK: "-ffp-contract=fast" ! CHECK: "-fconvert=little-endian" ! CHECK: "-mllvm" "-print-before-all" Index: flang/test/Driver/flang_fp_opts.f90 === --- /dev/null +++ flang/test/Driver/flang_fp_opts.f90 @@ -0,0 +1,5 @@ +! Test for passing of floating point options between the compiler and frontend +! drivers. + +! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s +! CHECK: ffp-contract= is not currently implemented Index: flang/test/Driver/driver-help.f90 === --- flang/test/Driver/driver-help.f90 +++ flang/test/Driver/driver-help.f90 @@ -31,6 +31,7 @@ ! HELP-NEXT: -ffixed-form Process source files in fixed form ! HELP-NEXT: -ffixed-line-length= ! HELP-NEXT: Use as character line width in fixed mode +! HELP-NEXT: -ffp-contract= 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. ! HELP-NEXT: -ffree-formProcess source files in free form ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! HELP-NEXT: -finput-charset= Specify the default character set for source files @@ -105,6 +106,7 @@ ! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form ! HELP-FC1-NEXT: -ffixed-line-length= ! HELP-FC1-NEXT: Use as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract= 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. ! HELP-FC1-NEXT: -ffree-formProcess source files in free form ! HELP-FC1-NEXT: -fget-definition ! HELP-FC1-NEXT:Get the symbol definition from Index: flang/test/Driver/driver-help-hidden.f90 === --- flang/test/Driver/driver-help-hidden.f90 +++ flang/test/Driver/driver-help-hidden.f90 @@ -31,6 +31,7 @@ ! CHECK-NEXT: -ffixed-form Process source files in fixed form ! CHECK-NEXT: -ffixed-line-length= ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= 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-formProcess source files in free form ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements ! CHECK-NEXT: -finput-charset= Specify the default character set for source files Index: flang/lib/Frontend/LangOptions.cpp === --- /dev/null +++ flang/lib/Frontend/LangOptions.cpp @@ -0,0 +1,24 @@ +//===-- LangOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Coding style:
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah marked 5 inline comments as done.
tblah added a comment.
Thanks for taking a look. I've updated accordingly and will upload the patch
soon.
> are you confident that we will need LangOptions.def?
Clang places this flag (and many other floating point options) in LangOptions
so I thought I would follow the same convention here. Is there somewhere more
appropriate to put them?
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:85-86
+ArgStringList ) {
+ // TODO: share RenderFloatingPointOptions from ./Clang.cpp and use that
+ // instead of duplicating code here
+ StringRef FPContract;
awarzynski wrote:
> What's RenderFloatingPointOptions?
It is a static function in clang/lib/Driver/ToolChains/Clang.cpp which does the
same job as AddFloatingPointOptions, except for clang. I couldn't use it right
away because not all of the options it it processes are supported in flang, but
once we get there it would make sense to share code.
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
[PATCH] D136080: [flang] Add -ffp-contract option processing
awarzynski added a comment.
Thanks for implementing this, @tblah!
Two high level questions/requests:
- are you confident that we will need LangOptions.def?
- can you upload a patch with full context? (some details can be found here:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)
-Andrzej
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:85-86
+ArgStringList ) {
+ // TODO: share RenderFloatingPointOptions from ./Clang.cpp and use that
+ // instead of duplicating code here
+ StringRef FPContract;
What's RenderFloatingPointOptions?
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:664
+///
+/// \param [out] res Stores the processed arguments
+/// \param [in] args The arguments to parse
`res` is a very confusing name (that I used myself in various places).
Basically, it's the `CompilerInvocation` instance ... result. Perhaps use
`invoc`?
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:665
+/// \param [out] res Stores the processed arguments
+/// \param [in] args The arguments to parse
+/// \param [out] diags DiagnosticsEngine to report erros with
[nit] "The compiler invocation args to parse"
Comment at: flang/test/Driver/driver-help.f90:108
! HELP-FC1-NEXT: Use as character line width in fixed mode
+! HELP-FC1-NEXT: -ffp-contract= 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.
! HELP-FC1-NEXT: -ffree-formProcess source files in free form
Why not expose this flag in `flang-new`? (as well as `flang-new -fc1`?)
Comment at: flang/test/Driver/flang_fp_opts.f90:1-2
+! Test for passing of floating point options between the compiler and frontend
+! drivers.
+
Sounds like a test for [[
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
| frontend-forwarding.f90 ]]
Comment at: flang/test/Driver/flang_fp_opts.f90:6-7
+
+! CHECK1-LABEL: "-fc1"
+! CHECK1: -ffp-contract=fast
+
Could you use more descriptive prefixes?
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
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah updated this revision to Diff 468258.
tblah added a comment.
clang-format plus cleaning up typos.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136080/new/
https://reviews.llvm.org/D136080
Files:
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Flang.cpp
flang/include/flang/Frontend/CompilerInvocation.h
flang/include/flang/Frontend/LangOptions.def
flang/include/flang/Frontend/LangOptions.h
flang/lib/Frontend/CMakeLists.txt
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/LangOptions.cpp
flang/test/Driver/driver-help.f90
flang/test/Driver/flang_fp_opts.f90
Index: flang/test/Driver/flang_fp_opts.f90
===
--- /dev/null
+++ flang/test/Driver/flang_fp_opts.f90
@@ -0,0 +1,10 @@
+! Test for passing of floating point options between the compiler and frontend
+! drivers.
+
+! RUN: %flang -### -S -ffp-contract=fast %s 2>&1 | FileCheck %s --check-prefix=CHECK1
+
+! CHECK1-LABEL: "-fc1"
+! CHECK1: -ffp-contract=fast
+
+! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s --check-prefix=CHECK2
+! CHECK2: ffp-contract= is not currently implemented
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -105,6 +105,7 @@
! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form
! HELP-FC1-NEXT: -ffixed-line-length=
! HELP-FC1-NEXT: Use as character line width in fixed mode
+! HELP-FC1-NEXT: -ffp-contract= 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.
! HELP-FC1-NEXT: -ffree-formProcess source files in free form
! HELP-FC1-NEXT: -fget-definition
! HELP-FC1-NEXT:Get the symbol definition from
Index: flang/lib/Frontend/LangOptions.cpp
===
--- /dev/null
+++ flang/lib/Frontend/LangOptions.cpp
@@ -0,0 +1,24 @@
+//===-- LangOptions.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
+//
+//===--===//
+
+#include "flang/Frontend/LangOptions.h"
+#include
+
+namespace Fortran::frontend {
+
+LangOptions::LangOptions() {
+#define LANGOPT(Name, Bits, Default) Name = Default;
+#define ENUM_LANGOPT(Name, Type, Bits, Default) set##Name(Default);
+#include "flang/Frontend/LangOptions.def"
+}
+
+} // end namespace Fortran::frontend
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -658,6 +658,46 @@
return diags.getNumErrors() == numErrorsBefore;
}
+/// Parses all floating point related arguments and populates the variables
+/// options accordingly. Returns false if new errors are generated.
+///
+/// \param [out] res Stores the processed arguments
+/// \param [in] args The arguments to parse
+/// \param [out] diags DiagnosticsEngine to report erros with
+static bool parseFloatingPointArgs(CompilerInvocation ,
+ llvm::opt::ArgList ,
+ clang::DiagnosticsEngine ) {
+ LangOptions = res.getLangOpts();
+ const unsigned diagUnimplemented = diags.getCustomDiagID(
+ clang::DiagnosticsEngine::Warning, "%0 is not currently implemented");
+
+ if (const llvm::opt::Arg *a =
+ args.getLastArg(clang::driver::options::OPT_ffp_contract)) {
+const llvm::StringRef val = a->getValue();
+enum LangOptions::FPModeKind fpContractMode;
+
+if (val.equals("on"))
+ fpContractMode = LangOptions::FPM_On;
+else if (val.equals("off"))
+ fpContractMode = LangOptions::FPM_Off;
+else if (val.equals("fast"))
+ fpContractMode = LangOptions::FPM_Fast;
+else if (val.equals("fast-honor-pragmas"))
+ fpContractMode = LangOptions::FPM_FastHonorPragmas;
+else {
+ diags.Report(clang::diag::err_drv_unsupported_option_argument)
+ << a->getOption().getName() << val;
+ return false;
+}
+
+// TODO: actually use the flag in codegen
+diags.Report(diagUnimplemented) << a->getOption().getName();
+opts.setFPContractMode(fpContractMode);
+ }
+
+ return
[PATCH] D136080: [flang] Add -ffp-contract option processing
tblah created this revision.
tblah added reviewers: kiranchandramohan, MatsPetersson, DavidTruby, rovka,
vzakhari.
tblah added a project: Flang.
Herald added a subscriber: jdoerfert.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added a project: All.
tblah requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
Only add the option processing and store the result. No attributes are
added to FIR yet.
The omission of the `fast-honor-pragmas` argument from the compiler
driver is deliberate. Clang only supports this argument when passed to
the frontend driver.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136080
Files:
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Flang.cpp
flang/include/flang/Frontend/CompilerInvocation.h
flang/include/flang/Frontend/LangOptions.def
flang/include/flang/Frontend/LangOptions.h
flang/lib/Frontend/CMakeLists.txt
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/LangOptions.cpp
flang/test/Driver/driver-help.f90
flang/test/Driver/flang_fp_opts.f90
Index: flang/test/Driver/flang_fp_opts.f90
===
--- /dev/null
+++ flang/test/Driver/flang_fp_opts.f90
@@ -0,0 +1,10 @@
+! Test for passing of floating point options between the compiler and frontend
+! drivers.
+
+! RUN: %flang -### -S -ffp-contract=fast %s 2>&1 | FileCheck %s --check-prefix=CHECK1
+
+! CHECK1-LABEL: "-fc1"
+! CHECK1: -ffp-contract=fast
+
+! RUN: %flang_fc1 -ffp-contract=fast %s 2>&1 | FileCheck %s --check-prefix=CHECK2
+! CHECK2: ffp-contract= is not currently implemented
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -105,6 +105,7 @@
! HELP-FC1-NEXT: -ffixed-form Process source files in fixed form
! HELP-FC1-NEXT: -ffixed-line-length=
! HELP-FC1-NEXT: Use as character line width in fixed mode
+! HELP-FC1-NEXT: -ffp-contract= 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.
! HELP-FC1-NEXT: -ffree-formProcess source files in free form
! HELP-FC1-NEXT: -fget-definition
! HELP-FC1-NEXT:Get the symbol definition from
Index: flang/lib/Frontend/LangOptions.cpp
===
--- /dev/null
+++ flang/lib/Frontend/LangOptions.cpp
@@ -0,0 +1,24 @@
+//===-- LangOptions.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
+//
+//===--===//
+
+#include "flang/Frontend/LangOptions.h"
+#include
+
+namespace Fortran::frontend {
+
+LangOptions::LangOptions() {
+#define LANGOPT(Name, Bits, Default) Name = Default;
+#define ENUM_LANGOPT(Name, Type, Bits, Default) set##Name(Default);
+#include "flang/Frontend/LangOptions.def"
+}
+
+} // end namespace Fortran::frontend
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -658,6 +658,47 @@
return diags.getNumErrors() == numErrorsBefore;
}
+/// Parses all floating point related arguments and populates the variables
+/// options accordingly. Returns false if new errors are generated.
+///
+/// \param [out] res Stores the processed arguments
+/// \param [in] args The arguments to parse
+/// \param [out] diags DiagnosticsEngine to report erros with
+static bool parseFloatingPointArgs(CompilerInvocation ,
+ llvm::opt::ArgList ,
+ clang::DiagnosticsEngine ) {
+ using Fortran::frontend::CodeGenOptions;
+ LangOptions = res.getLangOpts();
+ const unsigned diagUnimplemented = diags.getCustomDiagID(
+ clang::DiagnosticsEngine::Warning, "%0 is not currently implemented");
+
+ if (const llvm::opt::Arg *a =
+ args.getLastArg(clang::driver::options::OPT_ffp_contract)) {
+const llvm::StringRef val = a->getValue();
+enum LangOptions::FPModeKind fpContractMode;
+
+if (val.equals("on"))
+ fpContractMode = LangOptions::FPM_On;
+else if (val.equals("off"))
+
