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

Introduce assembly tests suite #58791

Merged
merged 3 commits into from Mar 20, 2019

Conversation

Projects
None yet
8 participants
@denzp
Copy link
Contributor

commented Feb 27, 2019

The change introduces a new test suite - Assembly tests. The motivation behind this is an ability to perform end-to-end codegen testing with LLVM backend. Turned out, NVPTX backend sometimes missing common Rust features (i128 and libcalls in the past, and still full atomics support) due to different reasons.

Prior to this change, basic NVPTX assembly tests were implemented within run-make suite. Now, it's easier to write additional and maintain existing tests for the target.

cc @gnzlbg @peterhj
cc @eddyb I adjusted mangling scheme expectation, so there is no need to change the tests for #57967

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Feb 27, 2019

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@gnzlbg

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

The only thing that worries me a bit is that updating LLVM might require updating many tests, but I guess that's also kind of the point.

In any case this looks much better than run-make and is pretty much what LLVM does anyways. As long as we add new assembly tests wisely, the direction of this PR LGTM.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

This looks good to me, thanks @denzp!

I agree with @gnzlbg that these could be tricky to update over time, but we've already got codegen tests which need updates periodically, so I don't think that these are any worse off than those.

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

📌 Commit 5c7ec6c has been approved by alexcrichton

@denzp

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

Very likely, having more and more "precise" tests to verify only smaller and basic parts would minimise the chance that backend changes do much damage. Let's hope so and write the cases wisely 🙂

@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

☔️ The latest upstream changes (presumably #58330) made this pull request unmergeable. Please resolve the merge conflicts.

@mark-i-m

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

@denzp Could you also document this in the Testing chapter of the rustc-guide please?

@denzp

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

I've solved conflicts and opened a PR in the rustc-guide: rust-lang/rustc-guide#289

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

📌 Commit 60f1644 has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

⌛️ Testing commit 60f1644 with merge 2fba954...

bors added a commit that referenced this pull request Mar 19, 2019

Auto merge of #58791 - denzp:asm-compile-tests, r=alexcrichton
Introduce assembly tests suite

The change introduces a new test suite - **Assembly** tests. The motivation behind this is an ability to perform end-to-end codegen testing with LLVM backend. Turned out, NVPTX backend sometimes missing common Rust features (`i128` and libcalls in the past, and still full atomics support) due to different reasons.

Prior to this change, basic NVPTX assembly tests were implemented within `run-make` suite. Now, it's easier to write additional and maintain existing tests for the target.

cc @gnzlbg @peterhj
cc @eddyb I adjusted mangling scheme expectation, so there is no need to change the tests for #57967
@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2019

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:02:26] Questions should be asked at https://answers.launchpad.net/gcc-arm-embedded
[00:02:26] 
[00:02:26] Bug can be filed at https://bugs.launchpad.net/gcc-arm-embedded/+filebug. It is highly encouraged to ask question first before filing a bug.
[00:02:26]  More info: https://launchpad.net/~team-gcc-arm-embedded/+archive/ubuntu/ppa
[00:02:26] gpg: keyring `/tmp/tmpz71y8vzt/secring.gpg' created
[00:02:26] gpg: keyring `/tmp/tmpz71y8vzt/pubring.gpg' created
[00:02:26] gpg: requesting key F64D33B0 from hkp server keyserver.ubuntu.com
[00:02:27] gpg: /tmp/tmpz71y8vzt/trustdb.gpg: trustdb created
[00:02:27] gpg: no ultimately trusted keys found
[00:02:27] gpg: Total number processed: 1
[00:02:27] gpg:               imported: 1  (RSA: 1)
[00:02:27] OK
---
travis_time:end:02590ac0:start=1552998402468279247,finish=1552998402478208338,duration=9929091
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2798a654
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:08e5b8e0
travis_time:start:08e5b8e0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0ff062a4
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@bors: retry

@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

⌛️ Testing commit 60f1644 with merge 2a1d774...

bors added a commit that referenced this pull request Mar 19, 2019

Auto merge of #58791 - denzp:asm-compile-tests, r=alexcrichton
Introduce assembly tests suite

The change introduces a new test suite - **Assembly** tests. The motivation behind this is an ability to perform end-to-end codegen testing with LLVM backend. Turned out, NVPTX backend sometimes missing common Rust features (`i128` and libcalls in the past, and still full atomics support) due to different reasons.

Prior to this change, basic NVPTX assembly tests were implemented within `run-make` suite. Now, it's easier to write additional and maintain existing tests for the target.

cc @gnzlbg @peterhj
cc @eddyb I adjusted mangling scheme expectation, so there is no need to change the tests for #57967
@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2019

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:02:21] Questions should be asked at https://answers.launchpad.net/gcc-arm-embedded
[00:02:21] 
[00:02:21] Bug can be filed at https://bugs.launchpad.net/gcc-arm-embedded/+filebug. It is highly encouraged to ask question first before filing a bug.
[00:02:21]  More info: https://launchpad.net/~team-gcc-arm-embedded/+archive/ubuntu/ppa
[00:02:21] gpg: keyring `/tmp/tmpbwn30ypb/secring.gpg' created
[00:02:21] gpg: keyring `/tmp/tmpbwn30ypb/pubring.gpg' created
[00:02:21] gpg: requesting key F64D33B0 from hkp server keyserver.ubuntu.com
[00:02:22] gpg: /tmp/tmpbwn30ypb/trustdb.gpg: trustdb created
[00:02:22] gpg: no ultimately trusted keys found
[00:02:22] gpg: Total number processed: 1
[00:02:22] gpg:               imported: 1  (RSA: 1)
[00:02:22] OK
---
travis_time:end:043e0e67:start=1553020200785936297,finish=1553020200794530088,duration=8593791
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0e55216c
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0c4770b2
travis_time:start:0c4770b2
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:14c08ec0
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

cc @jackpot51 it looks like the redox apt servers may be down? We're getting this on CI:

[00:10:58] �[91mW: The repository 'https://static.redox-os.org/toolchain/apt  Release' does not have a Release file.
[00:10:58] E: Failed to fetch https://static.redox-os.org/toolchain/apt/Packages  404  Not Found
[00:10:58] E: Some index files failed to download. They have been ignored, or old ones used instead.
[00:10:58] The command '/bin/sh -c ./install-x86_64-redox.sh' returned a non-zero code: 100
[00:11:02] �[0mCommand failed. Attempt 5/5:

Do you know if this could be fixed in the near future? If not no worries! We'll just need to disable the redox targets in nightly until it's fixed

@lzutao

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

cc #59257

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

Oh oops, thanks for the reference @lzutao!

In that case @denzp if you want to remove the redox target from the dist-various-1 container I think that's fine. We can then sort out the merge conflict if either this or #59257 lands first.

@denzp

This comment has been minimized.

Copy link
Contributor Author

commented Mar 19, 2019

@alexcrichton it's okay to wait with this PR until #59257 is merged. I guess it should happend pretty soon, because there is a rollup going on #59298.

@lzutao

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

#59257 is merged. Could this PR be unlocked?

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

@bors: retry

@bors bors added S-waiting-on-bors and removed S-blocked labels Mar 20, 2019

@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

⌛️ Testing commit 60f1644 with merge 82e2f3e...

bors added a commit that referenced this pull request Mar 20, 2019

Auto merge of #58791 - denzp:asm-compile-tests, r=alexcrichton
Introduce assembly tests suite

The change introduces a new test suite - **Assembly** tests. The motivation behind this is an ability to perform end-to-end codegen testing with LLVM backend. Turned out, NVPTX backend sometimes missing common Rust features (`i128` and libcalls in the past, and still full atomics support) due to different reasons.

Prior to this change, basic NVPTX assembly tests were implemented within `run-make` suite. Now, it's easier to write additional and maintain existing tests for the target.

cc @gnzlbg @peterhj
cc @eddyb I adjusted mangling scheme expectation, so there is no need to change the tests for #57967
@bors

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 82e2f3e to master...

@bors bors added the merged-by-bors label Mar 20, 2019

@bors bors merged commit 60f1644 into rust-lang:master Mar 20, 2019

1 check passed

homu Test successful
Details

@denzp denzp deleted the denzp:asm-compile-tests branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.