Skip to content

[OpenMP][test] XFAIL misc_bugs/many-microtask-args.c on SPARC #142385

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Jun 2, 2025

As detailed in Issue #139905, the libomp :: misc_bugs/many-microtask-args.c test currently FAILs on SPARC:

# | Too many args to microtask: 17!

This happens on every target that lacks an assembler implementation of __kmp_invoke_microtask, so this patch XFAILs the test on SPARC.

The target={{sparc.*}} syntax requires PR #142380.

Tested on sparc-sun-solaris2.11, sparcv9-sun-solaris2.11, sparc-unknown-linux-gnu, and sparc64-unknown-linux-gnu.

As detailed in Issue llvm#139905, the `libomp :: misc_bugs/many-microtask-args.c`
test currently `FAIL`s on SPARC:

```
# | Too many args to microtask: 17!

```
This happens on every target that lacks an assembler implementation of
`__kmp_invoke_microtask`, so this patch `XFAIL`s the test on SPARC.

The `target={{sparc.*}}` syntax requires PR llvm#142380.

Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`,
`sparc-unknown-linux-gnu`, and `sparc64-unknown-linux-gnu`.
@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Jun 2, 2025
Copy link
Collaborator

@jprotze jprotze left a comment

Choose a reason for hiding this comment

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

The test is expected to succeed for any OpenMP-compliant implementation.
Therefore I'm reluctant to mark the test as X-FAIL just for the purpose of silencing the test. The test highlights, that LLVM is not able to support correct OpenMP code for these platforms as long as the test fails.

As I tried to point out in the issue: Adding assembler code is not the only way to solve the issue.
The alternative is to modify the CodeGen to package all or just the remaining arguments into an array, pass them into the function as an array and modify the codegen for the outlined code block to unpack the array.

@rorth
Copy link
Collaborator Author

rorth commented Jun 4, 2025

The test is expected to succeed for any OpenMP-compliant implementation. Therefore I'm reluctant to mark the test as X-FAIL just for the purpose of silencing the test. The test highlights, that LLVM is not able to support correct OpenMP code for these platforms as long as the test fails.

As I tried to point out in the issue: Adding assembler code is not the only way to solve the issue. The alternative is to modify the CodeGen to package all or just the remaining arguments into an array, pass them into the function as an array and modify the codegen for the outlined code block to unpack the array.

In that case this has to be dropped:

  • I have close to no experience with codegen. 2 or 3 feeble attempts years ago only took excessive amounts of time, but ultimately led to nothing but a total waste of time. This is way out of my league.
  • I cannot tell when (or if) I'll be able to try an assembler implementation instead.

@jprotze
Copy link
Collaborator

jprotze commented Jun 4, 2025

The relevant Codegen function should be: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGStmtOpenMP.cpp#L1602

I don't really understand, how the code makes sure that emitParallelOutlinedFunction and emitParallelCall have a common understanding about the argument list. Probably @dhruvachak or @alexey-bataev can help to implement an alternative CodeGen which wraps exceeding arguments into an array? The alternative codegen should be chosen for architectures where the runtime does not support more than 17 arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants