Skip to content
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

Merged
merged 8 commits into from
May 14, 2024

Conversation

mjklemm
Copy link
Contributor

@mjklemm mjklemm commented May 2, 2024

This should be a NFC change for all drivers, but Flang.

@mjklemm mjklemm self-assigned this May 2, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category labels May 2, 2024
@llvmbot
Copy link
Member

llvmbot commented May 2, 2024

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Michael Klemm (mjklemm)

Changes

This should be a NFC change for all drivers, but Flang.


Full diff: https://github.com/llvm/llvm-project/pull/90886.diff

5 Files Affected:

  • (modified) clang/include/clang/Driver/Driver.h (+3)
  • (modified) clang/include/clang/Driver/Options.td (+1-1)
  • (modified) clang/lib/Driver/Driver.cpp (+20-3)
  • (modified) flang/test/Driver/driver-help-hidden.f90 (+1)
  • (added) flang/test/Driver/print-resource-dir.F90 (+3)
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]]

Copy link
Contributor

@banach-space banach-space left a 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?

@mjklemm
Copy link
Contributor Author

mjklemm commented May 3, 2024

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 lib and include directories. So, one level above of what I have done in this PR right now.

For reference, Clang outputs this:

$ which clang
clang is /usr/bin/clang
$ clang -print-resource-dir
/usr/lib/llvm-14/lib/clang/14.0.0
$ ls /usr/lib/llvm-14/lib/clang/14.0.0
total 36
24 include/   4 lib/   4 share/   4 README.txt

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.

@banach-space
Copy link
Contributor

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 lib and include directories. So, one level above of what I have done in this PR right now.

How does this compare to GFortran and Classic Flang? Anything resembling this flag?

@mjklemm
Copy link
Contributor Author

mjklemm commented May 3, 2024

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.

@banach-space
Copy link
Contributor

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 ;-)

@mjklemm
Copy link
Contributor Author

mjklemm commented May 3, 2024

Do you know the meaning for Classic Flang?

The way -print-resource-dir now works should be consistent with how Classic Flang works. If that's not the right response, I guess I got my wires crossed and you need to explain it to me. :-)

Copy link
Contributor

@banach-space banach-space left a 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 🙏🏻

@mjklemm
Copy link
Contributor Author

mjklemm commented May 3, 2024

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!

@mjklemm
Copy link
Contributor Author

mjklemm commented May 7, 2024

@tblah Ping :-)

@mjklemm mjklemm requested a review from Meinersbur May 10, 2024 11:49
Copy link
Contributor

@tblah tblah left a 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

@mjklemm mjklemm merged commit 2df06e4 into llvm:main May 14, 2024
4 checks passed
@mjklemm
Copy link
Contributor Author

mjklemm commented May 14, 2024

@tblah @banach-space Thanks for the reviews!

@DavidTruby
Copy link
Member

DavidTruby commented Jun 24, 2024

Hi @mjklemm, this appears to be breaking linking entirely on Windows, I'm not sure how it's taken us so long to notice...
With the patch I get errors trying to find compiler-rt but without it I can build fine. I wonder why this patch changed the resource directory, as well as making it printable with a flag?

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 :)

@mjklemm
Copy link
Contributor Author

mjklemm commented Jun 24, 2024

Yeah, go ahead and revert. I can create a new PR from it just print the resource dir.

DavidTruby added a commit that referenced this pull request Jun 24, 2024
…emit Flang's resource directory" (#96557)

Reverts #90886

These changes broke linking to compiler-rt on Windows
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
…emit Flang's resource directory" (llvm#96557)

Reverts llvm#90886

These changes broke linking to compiler-rt on Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants