-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[llvm-profdata] Reintroduce use of InitLLVM #164736
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
Conversation
Now that llvm-profdata does not participate in llvm-driver it must directly call InitLLVM again in order to expand wildcard arguments on Windows. This PR reintroduces the use of InitLLVM as it was before llvm-profdata was moved to llvm-driver and also adds a test for the wildcard expansion on Windows.
|
@llvm/pr-subscribers-pgo Author: Tim Creech (tcreech-intel) ChangesNow that llvm-profdata does not participate in llvm-driver it makes sense to directly call InitLLVM again in order to expand wildcard arguments on Windows. This change reintroduces the use of InitLLVM (as it was before llvm-profdata was moved to llvm-driver) and also adds a test for the wildcard expansion on Windows. Full diff: https://github.com/llvm/llvm-project/pull/164736.diff 2 Files Affected:
diff --git a/llvm/test/tools/llvm-profdata/input-wildcard.test b/llvm/test/tools/llvm-profdata/input-wildcard.test
new file mode 100644
index 0000000000000..f2c46c962a817
--- /dev/null
+++ b/llvm/test/tools/llvm-profdata/input-wildcard.test
@@ -0,0 +1,15 @@
+# This test verifies that llvm-profdata will do wildcard expansion on its
+# arguments. The expansion is done by Windows-specific support in InitLLVM, so
+# we only expect this to work on Windows hosts.
+# REQUIRES: system-windows
+
+# Create two files to glob.
+RUN: echo '# empty profile 1' > %t.prof1.proftxt
+RUN: echo '# empty profile 2' >> %t.prof2.proftxt
+
+# Prevent LIT itself from globbing by quoting the wildcard argument.
+RUN: llvm-profdata merge "%t.*.proftxt" -dump-input-file-list -o /dev/null | FileCheck %s
+
+# Verify that llvm-profdata expanded the wildcard argument.
+CHECK: 1,{{.*}}.prof1.proftxt
+CHECK-NEXT: 1,{{.*}}.prof2.proftxt
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 15ddb05f953ee..a356bcd0773e0 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -34,7 +34,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/FormattedStream.h"
-#include "llvm/Support/LLVMDriver.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/MD5.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -3465,6 +3465,7 @@ static int order_main() {
}
int main(int argc, const char *argv[]) {
+ InitLLVM X(argc, argv);
StringRef ProgName(sys::path::filename(argv[0]));
if (argc < 2) {
|
|
FWIW we want to support llvm-driver in llvm-profdata, but we first need to replace the use of |
|
@tcreech-intel -- As @petrhosek mentioned, I am working on porting llvm-profdata to OptTable so that it can be added back to the llvm driver though its a bit low on my priorities right now. Will it be alright with you if I remove this InitLLVM call when I make llvm-profdata part of the llvm-driver again? cc: @mingmingl-llvm |
|
Hi @Prabhuk: yes, that's completely fine by me. When you move back to llvm-driver an InitLLVM in llvm-profdata.cpp would be redundant with this one in llvm-driver-template.cpp: llvm-project/llvm/cmake/modules/llvm-driver-template.cpp.in Lines 15 to 18 in 83f751a
For my purposes I'd just like to ensure that a call to InitLLVM is preserved somewhere to do the expansion. The new test should help. |
|
Thank you @tcreech-intel |
Thanks, @petrhosek. I'll clarify the commit message a little to reflect that the current situation is likely temporary. |
Before llvm-profdata participated in llvm-driver it directly called InitLLVM, which takes care of wildcard argument expansion for tools on Windows. When llvm-driver support was added to llvm-profdata this InitLLVM call was effectively moved into the common llvm-driver wrapper mechanism. More recently, in llvm#162191, llvm-driver support was temporarily backed out of llvm-profdata due to an issue with `cl::opt` handling. This change reintroduces the direct call to InitLLVM in order to restore wildcard expansion and also adds a test for the wildcard expansion on Windows.
Before llvm-profdata participated in llvm-driver it directly called InitLLVM, which takes care of wildcard argument expansion for tools on Windows. When llvm-driver support was added to llvm-profdata this InitLLVM call was effectively moved into the common llvm-driver wrapper mechanism. More recently, in llvm#162191, llvm-driver support was temporarily backed out of llvm-profdata due to an issue with `cl::opt` handling. This change reintroduces the direct call to InitLLVM in order to restore wildcard expansion and also adds a test for the wildcard expansion on Windows.
Before llvm-profdata participated in llvm-driver it directly called InitLLVM, which takes care of wildcard argument expansion for tools on Windows. When llvm-driver support was added to llvm-profdata this InitLLVM call was effectively moved into the common llvm-driver wrapper mechanism. More recently, in llvm#162191, llvm-driver support was temporarily backed out of llvm-profdata due to an issue with `cl::opt` handling. This change reintroduces the direct call to InitLLVM in order to restore wildcard expansion and also adds a test for the wildcard expansion on Windows.
Before llvm-profdata participated in llvm-driver it directly called InitLLVM, which takes care of wildcard argument expansion for tools on Windows. When llvm-driver support was added to llvm-profdata this InitLLVM call was effectively moved into the common llvm-driver wrapper mechanism.
More recently, in #162191, llvm-driver support was temporarily backed out of llvm-profdata due to an issue with
cl::opthandling. This change reintroduces the direct call to InitLLVM in order to restore wildcard expansion and also adds a test for the wildcard expansion on Windows.