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

[MCJIT][test] Move MCJIT non-interpreter tests to MCJIT subdirectory #124463

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

asb
Copy link
Contributor

@asb asb commented Jan 26, 2025

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 (as the logic in MCJIT/lit.local.cfg should be sufficient).

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.


As noted above I've left the force-interpreter=true tests alone but welcome feedback on if they should be moved as well.

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`.
@asb asb requested review from lhames, wangleiat and zhaoqi5 January 26, 2025 13:17
asb added a commit to asb/llvm-project that referenced this pull request Jan 26, 2025
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

Copy link
Contributor

@lhames lhames left a comment

Choose a reason for hiding this comment

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

This looks like a nice cleanup.

@asb asb merged commit 62c16d8 into llvm:main Jan 28, 2025
9 checks passed
asb added a commit that referenced this pull request Jan 28, 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 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