Skip to content

compiletest powerpc64 arch is useless #47737

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
jcowgill opened this issue Jan 25, 2018 · 3 comments · Fixed by #48732
Closed

compiletest powerpc64 arch is useless #47737

jcowgill opened this issue Jan 25, 2018 · 3 comments · Fixed by #48732
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jcowgill
Copy link
Contributor

The ARCH_TABLE in compiletest contains a useless entry for powerpc64.
https://github.com/rust-lang/rust/blob/79b25c666fec/src/tools/compiletest/src/util.rs#L34

The only user of that table is get_arch which scans the table sequentially to see if a target contains an architecture in the table. Since powerpc is a substring of powerpc64, the entry for powerpc64 will never match anything and is therefore useless.

This also means that all ignore-powerpc64 headers in the testsuite currently do nothing. I also found a few ignore-powerpc64le headers which probably do nothing as well.

@cuviper cuviper added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. O-PowerPC Target: PowerPC processors labels Jan 27, 2018
@debris
Copy link
Contributor

debris commented Mar 4, 2018

What about arm and arm64? Aren't they also affected?

("arm", "arm"),
("arm64", "aarch64"),

@jcowgill
Copy link
Contributor Author

jcowgill commented Mar 4, 2018

Yes I think arm64 is affected as well. I'm not sure why we need both arm64 and aarch64.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Mar 4, 2018

It seems... ungreat for compiletest to ignore tests tagged ignore-powerpc on powerpc64 given that powerpc is apparently used as the name for the 32 bit architecture. Likewise for arm. #48732 seems to indicate all current tests that ignore "powerpc32" also ignore powerpc64, but if we ever have a test that should run on 64 bit PPC and not on 32 bit PPC, we'd be hosed. Edit: I guess {enable,ignore}-32bit might help? Not sure, and even if it works, would be inconsistent with x86/x86_64.

I think the right fix here is to make compiletest stricter (i.e., only match entire --separated triple components, not substrings).

Manishearth added a commit to Manishearth/rust that referenced this issue Mar 5, 2018
…hton

Remove useless powerpc64 entry from ARCH_TABLE

Hope, I understood the scope of the fix correctly. closes rust-lang#47737
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Mar 6, 2018
…hton

Remove useless powerpc64 entry from ARCH_TABLE

Hope, I understood the scope of the fix correctly. closes rust-lang#47737
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Mar 6, 2018
…hton

Remove useless powerpc64 entry from ARCH_TABLE

Hope, I understood the scope of the fix correctly. closes rust-lang#47737
kennytm added a commit to kennytm/rust that referenced this issue Mar 6, 2018
…hton

Remove useless powerpc64 entry from ARCH_TABLE

Hope, I understood the scope of the fix correctly. closes rust-lang#47737
varkor added a commit to varkor/rust that referenced this issue Mar 19, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants