-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory #90886
Conversation
This should be an NFC change for anythign, but Flang.
@llvm/pr-subscribers-flang-driver @llvm/pr-subscribers-clang Author: Michael Klemm (mjklemm) ChangesThis should be a NFC change for all drivers, but Flang. Full diff: https://github.com/llvm/llvm-project/pull/90886.diff 5 Files Affected:
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 2ffc52bcb7ad3b..c36595e62e2daf 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -752,6 +752,9 @@ class Driver {
/// option.
void setDriverMode(StringRef DriverModeValue);
+ /// Set the resource directory, depending on which driver is being used.
+ void setResourceDirectory();
+
/// Parse the \p Args list for LTO options and record the type of LTO
/// compilation based on which -f(no-)?lto(=.*)? option occurs last.
void setLTOMode(const llvm::opt::ArgList &Args);
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 864da4e1157f7d..a3b81fa338bdcd 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5474,7 +5474,7 @@ def print_prog_name_EQ : Joined<["-", "--"], "print-prog-name=">,
Visibility<[ClangOption, CLOption]>;
def print_resource_dir : Flag<["-", "--"], "print-resource-dir">,
HelpText<"Print the resource directory pathname">,
- Visibility<[ClangOption, CLOption]>;
+ Visibility<[ClangOption, CLOption, FlangOption]>;
def print_search_dirs : Flag<["-", "--"], "print-search-dirs">,
HelpText<"Print the paths used for finding libraries and programs">,
Visibility<[ClangOption, CLOption]>;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 114320f5d31468..d75d2e88fc1024 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -229,9 +229,6 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
UserConfigDir = static_cast<std::string>(P);
}
#endif
-
- // Compute the path to the resource directory.
- ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
}
void Driver::setDriverMode(StringRef Value) {
@@ -250,6 +247,25 @@ void Driver::setDriverMode(StringRef Value) {
Diag(diag::err_drv_unsupported_option_argument) << OptName << Value;
}
+void Driver::setResourceDirectory() {
+ // Compute the path to the resource directory, depending on the driver mode.
+ switch (Mode) {
+ case GCCMode:
+ case GXXMode:
+ case CPPMode:
+ case CLMode:
+ case DXCMode:
+ ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+ break;
+ case FlangMode:
+ // TODO: Is there a better way to add the "../include/flang/" component?
+ SmallString<64> relPath{};
+ llvm::sys::path::append(relPath, "..", "include", "flang");
+ ResourceDir = GetResourcesPath(ClangExecutable, relPath);
+ break;
+ }
+}
+
InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings,
bool UseDriverMode, bool &ContainsError) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
@@ -1202,6 +1218,7 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
if (!DriverMode.empty())
setDriverMode(DriverMode);
+ setResourceDirectory();
// FIXME: What are we going to do with -V and -b?
// Arguments specified in command line.
diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90
index 706b2cb6c2452c..73b34bd321c5f6 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -143,6 +143,7 @@
! CHECK-NEXT: -o <file> Write output to <file>
! CHECK-NEXT: -pedantic Warn on language extensions
! CHECK-NEXT: -print-effective-triple Print the effective target triple
+! CHECK-NEXT: -print-resource-dir Print the resource directory pathname
! CHECK-NEXT: -print-target-triple Print the normalized target triple
! CHECK-NEXT: -pthread Support POSIX threads in generated code
! CHECK-NEXT: -P Disable linemarker output in -E mode
diff --git a/flang/test/Driver/print-resource-dir.F90 b/flang/test/Driver/print-resource-dir.F90
new file mode 100644
index 00000000000000..5c934312e1f68a
--- /dev/null
+++ b/flang/test/Driver/print-resource-dir.F90
@@ -0,0 +1,3 @@
+! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \
+! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR -DFILE=%S/Inputs/resource_dir %s
+! PRINT-RESOURCE-DIR: [[FILE]]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the definition of "resource dir" for Fortran?
I'd like to at least have it point to where the MODULE files live. When I look at what clang emits, then Flang's resource directory should rather point to the place, where Flang has its For reference, Clang outputs this:
At some point, I guess Flang might also have to switch to a scheme that includes version numbers to differentiate different resource directories, so that multiple different versions of Flang can coexist. But that's for a later PR. |
How does this compare to GFortran and Classic Flang? Anything resembling this flag? |
GFortran does not have it, but Classic Flang does. So, it's closing a gap to Classic Flang here as well. |
Do you know the meaning for Classic Flang? Would be great to make "LLVM Flang" consistent with any "prior art". Nice to have ;-) |
The way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for working on this and for addressing my comments 🙏🏻
More than happy to! Still learning how to be a proper Flang developer, so I'm thankful for all the comments! |
@tblah Ping :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for losing track of this, I was on holiday all of last week.
This LGTM
@tblah @banach-space Thanks for the reviews! |
Hi @mjklemm, this appears to be breaking linking entirely on Windows, I'm not sure how it's taken us so long to notice... Would you be amenable to reverting this while it can be fixed? I realise that it's awkward to ask for a speedy revert after waiting over a month but I'd rather not leave Windows broken even longer :) |
Yeah, go ahead and revert. I can create a new PR from it just print the resource dir. |
…emit Flang's resource directory" (llvm#96557) Reverts llvm#90886 These changes broke linking to compiler-rt on Windows
This should be a NFC change for all drivers, but Flang.