Allow disabling of types from the command line #107126
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Renaud-K
requested review from
DavidTruby,
banach-space,
jeanPerier,
klausler and
tblah
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Renaud Kauffmann (Renaud-K) ChangesI would to add hidden options to disable types through the I am planning to file a couple of issues following this patch. Full diff: https://github.com/llvm/llvm-project/pull/107126.diff 5 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 83cf753e824845..8bc47fea5196e7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6761,6 +6761,12 @@ def fdefault_integer_8 : Flag<["-"],"fdefault-integer-8">, Group<f_Group>,
HelpText<"Set the default integer and logical kind to an 8 byte wide type">;
def fdefault_real_8 : Flag<["-"],"fdefault-real-8">, Group<f_Group>,
HelpText<"Set the default real kind to an 8 byte wide type">;
+def fdisable_real_3 : Flag<["-"],"fdisable-real-3">, Group<f_Group>,
+ Flags<[HelpHidden]>;
+def fdisable_real_10 : Flag<["-"],"fdisable-real-10">, Group<f_Group>,
+ Flags<[HelpHidden]>;
+def fdisable_integer_16 : Flag<["-"],"fdisable-integer-16">, Group<f_Group>,
+ Flags<[HelpHidden]>;
def flarge_sizes : Flag<["-"],"flarge-sizes">, Group<f_Group>,
HelpText<"Use INTEGER(KIND=8) for the result type in size-related intrinsics">;
diff --git a/flang/include/flang/Frontend/TargetOptions.h b/flang/include/flang/Frontend/TargetOptions.h
index fa72c77a028a1c..b0b64ae583d16c 100644
--- a/flang/include/flang/Frontend/TargetOptions.h
+++ b/flang/include/flang/Frontend/TargetOptions.h
@@ -38,6 +38,12 @@ class TargetOptions {
/// The list of target specific features to enable or disable, as written on
/// the command line.
std::vector<std::string> featuresAsWritten;
+
+ /// The real KINDs disabled for this target
+ std::vector<int> disabledRealKinds;
+
+ /// The integer KINDs disabled for this target
+ std::vector<int> disabledIntegerKinds;
};
} // end namespace Fortran::frontend
diff --git a/flang/include/flang/Tools/TargetSetup.h b/flang/include/flang/Tools/TargetSetup.h
index 238d66c9241dd0..b112b6b4d80578 100644
--- a/flang/include/flang/Tools/TargetSetup.h
+++ b/flang/include/flang/Tools/TargetSetup.h
@@ -10,6 +10,7 @@
#define FORTRAN_TOOLS_TARGET_SETUP_H
#include "flang/Evaluate/target.h"
+#include "flang/Frontend/TargetOptions.h"
#include "llvm/Target/TargetMachine.h"
namespace Fortran::tools {
@@ -17,6 +18,7 @@ namespace Fortran::tools {
[[maybe_unused]] inline static void setUpTargetCharacteristics(
Fortran::evaluate::TargetCharacteristics &targetCharacteristics,
const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
const std::string &compilerVersion, const std::string &compilerOptions) {
const llvm::Triple &targetTriple{targetMachine.getTargetTriple()};
@@ -24,7 +26,14 @@ namespace Fortran::tools {
if (targetTriple.getArch() != llvm::Triple::ArchType::x86_64)
targetCharacteristics.DisableType(
Fortran::common::TypeCategory::Real, /*kind=*/10);
-
+ for (auto realKind : targetOptions.disabledRealKinds) {
+ targetCharacteristics.DisableType(
+ common::TypeCategory::Real, realKind);
+ }
+ for (auto intKind : targetOptions.disabledIntegerKinds) {
+ targetCharacteristics.DisableType(
+ common::TypeCategory::Integer, intKind);
+ }
targetCharacteristics.set_compilerOptionsString(compilerOptions)
.set_compilerVersionString(compilerVersion);
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..5676947c2b1016 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -438,8 +438,19 @@ static void parseTargetArgs(TargetOptions &opts, llvm::opt::ArgList &args) {
for (const llvm::opt::Arg *currentArg :
args.filtered(clang::driver::options::OPT_target_feature))
opts.featuresAsWritten.emplace_back(currentArg->getValue());
-}
+ if (args.hasArg(clang::driver::options::OPT_fdisable_real_10)) {
+ opts.disabledRealKinds.push_back(10);
+ }
+
+ if (args.hasArg(clang::driver::options::OPT_fdisable_real_3)) {
+ opts.disabledRealKinds.push_back(3);
+ }
+
+ if (args.hasArg(clang::driver::options::OPT_fdisable_integer_16)) {
+ opts.disabledIntegerKinds.push_back(16);
+ }
+}
// Tweak the frontend configuration based on the frontend action
static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
if (opts.programAction == DebugDumpParsingLog)
@@ -1529,8 +1540,8 @@ CompilerInvocation::getSemanticsCtx(
std::string compilerVersion = Fortran::common::getFlangFullVersion();
Fortran::tools::setUpTargetCharacteristics(
- semanticsContext->targetCharacteristics(), targetMachine, compilerVersion,
- allCompilerInvocOpts);
+ semanticsContext->targetCharacteristics(), targetMachine, getTargetOpts(),
+ compilerVersion, allCompilerInvocOpts);
return semanticsContext;
}
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 736d68219581dd..279b04055798d0 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -18,6 +18,7 @@
#include "flang/Common/OpenMP-features.h"
#include "flang/Common/Version.h"
#include "flang/Common/default-kinds.h"
+#include "flang/Frontend/TargetOptions.h"
#include "flang/Lower/Bridge.h"
#include "flang/Lower/PFTBuilder.h"
#include "flang/Lower/Support/Verifier.h"
@@ -556,8 +557,8 @@ int main(int argc, char **argv) {
std::string compilerVersion = Fortran::common::getFlangToolFullVersion("bbc");
std::string compilerOptions = "";
Fortran::tools::setUpTargetCharacteristics(
- semanticsContext.targetCharacteristics(), *targetMachine, compilerVersion,
- compilerOptions);
+ semanticsContext.targetCharacteristics(), *targetMachine,
+ {}, compilerVersion, compilerOptions);
return mlir::failed(
convertFortranSourceToMLIR(inputFilename, options, programPrefix,
|
Member
|
@llvm/pr-subscribers-flang-driver Author: Renaud Kauffmann (Renaud-K) ChangesI would to add hidden options to disable types through the I am planning to file a couple of issues following this patch. Full diff: https://github.com/llvm/llvm-project/pull/107126.diff 5 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 83cf753e824845..8bc47fea5196e7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6761,6 +6761,12 @@ def fdefault_integer_8 : Flag<["-"],"fdefault-integer-8">, Group<f_Group>,
HelpText<"Set the default integer and logical kind to an 8 byte wide type">;
def fdefault_real_8 : Flag<["-"],"fdefault-real-8">, Group<f_Group>,
HelpText<"Set the default real kind to an 8 byte wide type">;
+def fdisable_real_3 : Flag<["-"],"fdisable-real-3">, Group<f_Group>,
+ Flags<[HelpHidden]>;
+def fdisable_real_10 : Flag<["-"],"fdisable-real-10">, Group<f_Group>,
+ Flags<[HelpHidden]>;
+def fdisable_integer_16 : Flag<["-"],"fdisable-integer-16">, Group<f_Group>,
+ Flags<[HelpHidden]>;
def flarge_sizes : Flag<["-"],"flarge-sizes">, Group<f_Group>,
HelpText<"Use INTEGER(KIND=8) for the result type in size-related intrinsics">;
diff --git a/flang/include/flang/Frontend/TargetOptions.h b/flang/include/flang/Frontend/TargetOptions.h
index fa72c77a028a1c..b0b64ae583d16c 100644
--- a/flang/include/flang/Frontend/TargetOptions.h
+++ b/flang/include/flang/Frontend/TargetOptions.h
@@ -38,6 +38,12 @@ class TargetOptions {
/// The list of target specific features to enable or disable, as written on
/// the command line.
std::vector<std::string> featuresAsWritten;
+
+ /// The real KINDs disabled for this target
+ std::vector<int> disabledRealKinds;
+
+ /// The integer KINDs disabled for this target
+ std::vector<int> disabledIntegerKinds;
};
} // end namespace Fortran::frontend
diff --git a/flang/include/flang/Tools/TargetSetup.h b/flang/include/flang/Tools/TargetSetup.h
index 238d66c9241dd0..b112b6b4d80578 100644
--- a/flang/include/flang/Tools/TargetSetup.h
+++ b/flang/include/flang/Tools/TargetSetup.h
@@ -10,6 +10,7 @@
#define FORTRAN_TOOLS_TARGET_SETUP_H
#include "flang/Evaluate/target.h"
+#include "flang/Frontend/TargetOptions.h"
#include "llvm/Target/TargetMachine.h"
namespace Fortran::tools {
@@ -17,6 +18,7 @@ namespace Fortran::tools {
[[maybe_unused]] inline static void setUpTargetCharacteristics(
Fortran::evaluate::TargetCharacteristics &targetCharacteristics,
const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
const std::string &compilerVersion, const std::string &compilerOptions) {
const llvm::Triple &targetTriple{targetMachine.getTargetTriple()};
@@ -24,7 +26,14 @@ namespace Fortran::tools {
if (targetTriple.getArch() != llvm::Triple::ArchType::x86_64)
targetCharacteristics.DisableType(
Fortran::common::TypeCategory::Real, /*kind=*/10);
-
+ for (auto realKind : targetOptions.disabledRealKinds) {
+ targetCharacteristics.DisableType(
+ common::TypeCategory::Real, realKind);
+ }
+ for (auto intKind : targetOptions.disabledIntegerKinds) {
+ targetCharacteristics.DisableType(
+ common::TypeCategory::Integer, intKind);
+ }
targetCharacteristics.set_compilerOptionsString(compilerOptions)
.set_compilerVersionString(compilerVersion);
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..5676947c2b1016 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -438,8 +438,19 @@ static void parseTargetArgs(TargetOptions &opts, llvm::opt::ArgList &args) {
for (const llvm::opt::Arg *currentArg :
args.filtered(clang::driver::options::OPT_target_feature))
opts.featuresAsWritten.emplace_back(currentArg->getValue());
-}
+ if (args.hasArg(clang::driver::options::OPT_fdisable_real_10)) {
+ opts.disabledRealKinds.push_back(10);
+ }
+
+ if (args.hasArg(clang::driver::options::OPT_fdisable_real_3)) {
+ opts.disabledRealKinds.push_back(3);
+ }
+
+ if (args.hasArg(clang::driver::options::OPT_fdisable_integer_16)) {
+ opts.disabledIntegerKinds.push_back(16);
+ }
+}
// Tweak the frontend configuration based on the frontend action
static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
if (opts.programAction == DebugDumpParsingLog)
@@ -1529,8 +1540,8 @@ CompilerInvocation::getSemanticsCtx(
std::string compilerVersion = Fortran::common::getFlangFullVersion();
Fortran::tools::setUpTargetCharacteristics(
- semanticsContext->targetCharacteristics(), targetMachine, compilerVersion,
- allCompilerInvocOpts);
+ semanticsContext->targetCharacteristics(), targetMachine, getTargetOpts(),
+ compilerVersion, allCompilerInvocOpts);
return semanticsContext;
}
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 736d68219581dd..279b04055798d0 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -18,6 +18,7 @@
#include "flang/Common/OpenMP-features.h"
#include "flang/Common/Version.h"
#include "flang/Common/default-kinds.h"
+#include "flang/Frontend/TargetOptions.h"
#include "flang/Lower/Bridge.h"
#include "flang/Lower/PFTBuilder.h"
#include "flang/Lower/Support/Verifier.h"
@@ -556,8 +557,8 @@ int main(int argc, char **argv) {
std::string compilerVersion = Fortran::common::getFlangToolFullVersion("bbc");
std::string compilerOptions = "";
Fortran::tools::setUpTargetCharacteristics(
- semanticsContext.targetCharacteristics(), *targetMachine, compilerVersion,
- compilerOptions);
+ semanticsContext.targetCharacteristics(), *targetMachine,
+ {}, compilerVersion, compilerOptions);
return mlir::failed(
convertFortranSourceToMLIR(inputFilename, options, programPrefix,
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
klausler
approved these changes
Sep 3, 2024
| HelpText<"Set the default integer and logical kind to an 8 byte wide type">; | ||
| def fdefault_real_8 : Flag<["-"],"fdefault-real-8">, Group<f_Group>, | ||
| HelpText<"Set the default real kind to an 8 byte wide type">; | ||
| def fdisable_real_3 : Flag<["-"],"fdisable-real-3">, Group<f_Group>, |
Contributor
There was a problem hiding this comment.
maybe also -fdisable-real-2?
banach-space
reviewed
Sep 3, 2024
Contributor
banach-space
left a comment
banach-space
left a comment
There was a problem hiding this comment.
What's the end goal here? To add an option for every possible bit-width? If yes, then these options should take bit width as parameters.
Btw, does GFortran implement anything like this? If yes, are you making sure that the semantics in Flang are consistent? Please also add help text.
Contributor
Author
|
The end goal is testing for issues that this PR would expose and that I am planning on filing next. |
vzakhari
approved these changes
Sep 4, 2024
Contributor
vzakhari
left a comment
vzakhari
left a comment
There was a problem hiding this comment.
Looks good to me, but maybe we can have just a single option that accepts predefined words like real2, integer4, etc. I wonder if we can process multiple instances of an option like
tblah
approved these changes
Sep 4, 2024
Contributor
tblah
left a comment
tblah
left a comment
There was a problem hiding this comment.
I agree with other reviewers that a generic option would be better than a list of particular types. But for me that's just a nit because these are hidden debug options anyway
Contributor
Author
|
I have been looking into this but long story short, but I am little pressed by time and was not planning on spending too much time on this. For the need that we have, it should be ok. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I would to add hidden options to disable types through the
TargetCharacteristics. I am seeing issues when I do this programmatically and would like, for anyone, to have the ability to reproduce them for development and testing purposes.I am planning to file a couple of issues following this patch.