-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add LLVM ModulePass regression test using run-make. #31391
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
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
||
all: | ||
$(CXX) $(LLVM_CXXFLAGS) -c llvm-pass.so.cc -o $(TMPDIR)/llvm-pass.o | ||
$(AR) rcs $(TMPDIR)/libllvm-pass.a $(TMPDIR)/llvm-pass.o |
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.
Can this use the support in tools.mk
? The all
target can depend on $(call NATIVE_STATICLIB,llvm-pass)
and then you can define a rule for $(TMPDIR)/llvm-pass.o
which depends on llvm-pass.so.cc
.
This may end up being pretty difficult to get past all the platforms like MinGW and MSVC, but in principle seems fine to me! |
@alexcrichton Addressed your comments. I'm not super familiar with conventions for |
$(RUSTC) main.irs | ||
|
||
$(TMPDIR)/libllvm-pass.a: $(TMPDIR)/llvm-pass.o | ||
$(AR) rcs $(TMPDIR)/libllvm-pass.a $(TMPDIR)/llvm-pass.o |
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.
You should be able to omit this as NATIVE_STATICLIB
above it should handle assembling this on platforms like MSVC where lib.exe
is used.
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.
/Users/coreyf/Development/rust/rust/src/test/run-make/llvm-module-pass/Makefile
diff --git a/src/test/run-make/llvm-module-pass/Makefile b/src/test/run-make/llvm-module-pass/Makefile
index 69e4d00..fd34bd2 100644
--- a/src/test/run-make/llvm-module-pass/Makefile
+++ b/src/test/run-make/llvm-module-pass/Makefile
@@ -5,7 +5,6 @@ all: $(call NATIVE_STATICLIB,llvm-pass)
$(RUSTC) main.irs
$(TMPDIR)/libllvm-pass.a: $(TMPDIR)/llvm-pass.o
- $(AR) rcs $(TMPDIR)/libllvm-pass.a $(TMPDIR)/llvm-pass.o
$(TMPDIR)/llvm-pass.o:
$(CXX) $(LLVM_CXXFLAGS) -c llvm-pass.so.cc -o $(TMPDIR)/llvm-pass.o
coreyf@frewbook-pro ~/D/r/rust (test)> make check-stage2-rmake
cfg: version 1.8.0-dev (6de413585 2016-02-03)
cfg: build triple x86_64-apple-darwin
cfg: host triples x86_64-apple-darwin
cfg: target triples x86_64-apple-darwin
cfg: disabling rustc optimization (CFG_DISABLE_OPTIMIZE)
cfg: enabling debug assertions (CFG_ENABLE_DEBUG_ASSERTIONS)
cfg: enabling debuginfo (CFG_ENABLE_DEBUGINFO)
cfg: host for x86_64-apple-darwin is x86_64
cfg: os for x86_64-apple-darwin is apple-darwin
cfg: disabling C++ optimization (CFG_DISABLE_OPTIMIZE_CXX)
cfg: have good valgrind for x86_64-apple-darwin
cfg: using CC=clang (CFG_CC)
cfg: using CXX=clang++ (CFG_CXX)
cfg: disabling valgrind run-pass tests
cfg: disabling doc build (CFG_DISABLE_DOCS)
cfg: including test rules
cfg: lexer tooling not available, skipping lexer test...
maketest: llvm-module-pass
----- /Users/coreyf/Development/rust/rust/src/test/run-make/llvm-module-pass/ --------------------
------ stdout ---------------------------------------------
c++ -I/Users/coreyf/Development/rust/rust/src/llvm/include -I/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/llvm/include -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -O3 -stdlib=libc++ -std=c++11 -fvisibility-inlines-hidden -fno-exceptions -fno-rtti -fno-common -Wcast-qual -c llvm-pass.so.cc -o /Users/coreyf/Development/rust/rust/x86_64-apple-darwin/test/run-make/llvm-module-pass/llvm-pass.o
DYLD_LIBRARY_PATH="/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/test/run-make/llvm-module-pass:/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/stage2/lib:/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/llvm/Release+Asserts/lib:" /Users/coreyf/Development/rust/rust/x86_64-apple-darwin/stage2/bin/rustc --out-dir /Users/coreyf/Development/rust/rust/x86_64-apple-darwin/test/run-make/llvm-module-pass -L /Users/coreyf/Development/rust/rust/x86_64-apple-darwin/test/run-make/llvm-module-pass -L native=/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/llvm/Release+Asserts/lib plugin.rs -C prefer-dynamic
------ stderr ---------------------------------------------
error: could not find native static library `llvm-pass`, perhaps an -L flag is missing?
make[1]: *** [all] Error 101
------ ---------------------------------------------
make: *** [x86_64-apple-darwin/test/run-make/llvm-module-pass-2-T-x86_64-apple-darwin-H-x86_64-apple-darwin.ok] Error 2
or maybe you meant
diff --git a/src/test/run-make/llvm-module-pass/Makefile b/src/test/run-make/llvm-module-pass/Makefile
index 69e4d00..34e8664 100644
--- a/src/test/run-make/llvm-module-pass/Makefile
+++ b/src/test/run-make/llvm-module-pass/Makefile
@@ -4,8 +4,5 @@ all: $(call NATIVE_STATICLIB,llvm-pass)
$(RUSTC) plugin.rs -C prefer-dynamic
$(RUSTC) main.irs
-$(TMPDIR)/libllvm-pass.a: $(TMPDIR)/llvm-pass.o
- $(AR) rcs $(TMPDIR)/libllvm-pass.a $(TMPDIR)/llvm-pass.o
-
$(TMPDIR)/llvm-pass.o:
$(CXX) $(LLVM_CXXFLAGS) -c llvm-pass.so.cc -o $(TMPDIR)/llvm-pass.o
coreyf@frewbook-pro ~/D/r/rust (test) [2]> make check-stage2-rmake
cfg: version 1.8.0-dev (6de413585 2016-02-03)
cfg: build triple x86_64-apple-darwin
cfg: host triples x86_64-apple-darwin
cfg: target triples x86_64-apple-darwin
cfg: disabling rustc optimization (CFG_DISABLE_OPTIMIZE)
cfg: enabling debug assertions (CFG_ENABLE_DEBUG_ASSERTIONS)
cfg: enabling debuginfo (CFG_ENABLE_DEBUGINFO)
cfg: host for x86_64-apple-darwin is x86_64
cfg: os for x86_64-apple-darwin is apple-darwin
cfg: disabling C++ optimization (CFG_DISABLE_OPTIMIZE_CXX)
cfg: have good valgrind for x86_64-apple-darwin
cfg: using CC=clang (CFG_CC)
cfg: using CXX=clang++ (CFG_CXX)
cfg: disabling valgrind run-pass tests
cfg: disabling doc build (CFG_DISABLE_DOCS)
cfg: including test rules
cfg: lexer tooling not available, skipping lexer test...
maketest: llvm-module-pass
----- /Users/coreyf/Development/rust/rust/src/test/run-make/llvm-module-pass/ --------------------
------ stdout ---------------------------------------------
------ stderr ---------------------------------------------
make[1]: *** No rule to make target `/Users/coreyf/Development/rust/rust/x86_64-apple-darwin/test/run-make/llvm-module-pass/libllvm-pass.a', needed by `all'. Stop.
------ ---------------------------------------------
make: *** [x86_64-apple-darwin/test/run-make/llvm-module-pass-2-T-x86_64-apple-darwin-H-x86_64-apple-darwin.ok] Error 2
Comments have been addressed |
|
⌛ Testing commit ffcba80 with merge 70bf97f... |
@bors r- (requested by the author) |
Should all the builders have LLVM headers set up? |
⛄ The build was interrupted to prioritize another pull request. |
Yeah all the builders should have LLVM set up correctly, but this is likely due to Windows paths and MSYS shell escaping, it looks like one of the arguments is:
But all those back slashes are interpreted as escapes, so I think it's actually looking for the path:
Which of course doesn't exist, hence the error :( |
@alexcrichton Do you have any suggestions on potential next steps for fixing that? I'm really not that familiar with linkers on Windows, let along linking in general |
Unfortunately I don't really know of a great way to deal with this. Nothing is fundamentally different on Windows, really, it's just that shell-on-windows doesn't work very well unless you're 100% staying within that shell (but we're taking input from the outside) For now since this is just a regression test, and I expect this to behavior pretty uniformly across platforms, let's just ignore this test on Windows? |
Is there some special way of ignoring platforms for a test case without a cargo feature flag? |
somewhere around the top of the test. (though that might not work for maketests?) |
Ah unfortunately that doesn't work for run-make tests, you'll have to do something like this in the makefile: ifdef IS_WINDOWS
all:
else
all:
...
endif |
💔 Test failed - auto-mac-32-opt |
So anyone have any ideas why this fails on OSX 32 bit? Intermittent? I'm running OSX 64 bit and just confirmed this test passes for me |
That error looks like something-or-other failed to have the plugin to load on OSX, it may be a 32/64 issue, but it doesn't look spurious |
I unfortunately don't have too much time right now to look into the 32 bit issue. Is it possible to just enable this test to run on 64 bit machines? |
Ok, debugged a bit, looks like you just need to pass in the normal CFLAGS as well. The |
Force pushed with the flag and rebased. Change I made: diff --git a/src/test/run-make/llvm-module-pass/Makefile b/src/test/run-make/llvm-module-pass/Makefile
index 339dd1c..0f457e7 100644
--- a/src/test/run-make/llvm-module-pass/Makefile
+++ b/src/test/run-make/llvm-module-pass/Makefile
@@ -10,5 +10,5 @@ all: $(call NATIVE_STATICLIB,llvm-pass)
$(RUSTC) main.rs
$(TMPDIR)/libllvm-pass.o:
- $(CXX) $(LLVM_CXXFLAGS) -c llvm-pass.so.cc -o $(TMPDIR)/libllvm-pass.o
+ $(CXX) $(LLVM_CFLAGS) $(LLVM_CXXFLAGS) -c llvm-pass.so.cc -o $(TMPDIR)/libllvm-pass.o
endif |
Also, thanks a ton for debugging the issue for me :) |
Hm, I'm not sure that It looks like we pass |
Updated to use |
I think you'll also need to set CFLAGS in the maketest.py script as right now |
Sorry! Not too familiar with the test+build process here. Let me know how that looks |
Part of rust-lang#31185. Similar to rust-lang#31391.
…est, r=alexcrichton Part of rust-lang#31185. Similar to rust-lang#31391.
Part of #31185