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

[ExecutionEngine][test][RISCV] Don't mark RISCV as unsupported #124464

Merged

Conversation

asb
Copy link
Contributor

@asb asb commented Jan 26, 2025

Although MCJIT is unsupported, we can and should be running the other
tests. Stacks on top of #124463 (needed to avoid unsupported MCJIT tests
in the top-level test/ExecutionEngine directory running when they
shouldn't).

This effectively reverts b8feeba.

ninja check-llvm-executionengine before:
Total Discovered Tests: 335
Unsupported: 335 (100.00%)

ninja check-llvm-executionengine after:
Total Discovered Tests: 335
Unsupported : 125 (37.31%)
Passed : 209 (62.39%)
Expectedly Failed: 1 (0.30%)

asb added 2 commits January 26, 2025 13:11
Moving to the MCJIT subdirectory means they can be gated by a common
lit.local.cfg. I remove the `; UNSUPPORTED: target=loongarch{{.*}}`
lines because of this.

The move is motivated by my desire to enable more of these tests for
RISCV, and it seems like it would be wrong to keep extending the
`UNSUPPORTED` lines for these individual tests.

This patch does not move the MCJIT tests in the top-level directory that
do `-force-interpreter=true`.
Although MCJIT is unsupported, we can and should be running the other
tests. Stacks on top of llvm#124463 (needed to avoid unsupported MCJIT tests
in the top-level test/ExecutionEngine directory running when they
shouldn't).

This effectively reverts b8feeba.

ninja check-llvm-executionengine before:
Total Discovered Tests: 335
  Unsupported: 335 (100.00%)

ninja check-llvm-executionengine after:
Total Discovered Tests: 335
  Unsupported      : 125 (37.31%)
  Passed           : 209 (62.39%)
  Expectedly Failed:   1 (0.30%)
Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

As a follow up, would you mind moving the force-interpreter tests into the Interpreter subtree?

@asb asb merged commit 606cf88 into llvm:main Jan 28, 2025
9 checks passed
asb added a commit to asb/llvm-project that referenced this pull request Jan 28, 2025
…subdirectory

I left these alone in llvm#124463 but I think it makes sense to clean these
up as well (which Philip also noted in llvm#124464).
asb added a commit that referenced this pull request Jan 29, 2025
…subdirectory (#124744)

I left these alone in #124463 but I think it makes sense to clean these
up as well (which Philip also noted in #124464).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants