Skip to content

Conversation

@rorth
Copy link
Collaborator

@rorth rorth commented May 5, 2025

libomp.so currently fails to link on SPARC, both Solaris/sparcv9 and Linux/sparc64:

Undefined                       first referenced
 symbol                             in file
__kmp_unnamed_critical_addr         projects/openmp/runtime/src/CMakeFiles/omp.dir/kmp_gsupport.cpp.o
ld: fatal: symbol referencing errors

This patch provides the necessary definition. While at it, I noticed that on non-x86 targets the symbol wasn't marked as @object, which this patch corrects, too.

Tested on sparcv9-sun-solaris2.11, sparc64-unknown-linux-gnu, amd64-pc-solaris2.11, and x86_64-pc-linux-gnu.

`libomp.so` currently fails to link on SPARC, both Solaris/sparcv9 and
Linux/sparc64:

```
Undefined                       first referenced
 symbol                             in file
__kmp_unnamed_critical_addr         projects/openmp/runtime/src/CMakeFiles/omp.dir/kmp_gsupport.cpp.o
ld: fatal: symbol referencing errors
```

This patch provides the necessary definition.  While at it, I noticed that
on non-x86 targets the symbol wasn't marked as `@object`, which this patch
corrects, too.

Tested on `sparcv9-sun-solaris2.11`, `sparc64-unknown-linux-gnu`,
`amd64-pc-solaris2.11`, and `x86_64-pc-linux-gnu`.
@rorth rorth requested review from brad0 and mstorsjo May 5, 2025 12:23
@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label May 5, 2025
@rorth rorth merged commit c24b9ca into llvm:main May 6, 2025
11 checks passed
@Sharjeel-Khan
Copy link
Contributor

Sharjeel-Khan commented May 6, 2025

This change caused an error in Android's LLVM builders:

[5/35] Building ASM object runtime/src/CMakeFiles/omp.dir/z_Linux_asm.S.o
FAILED: runtime/src/CMakeFiles/omp.dir/z_Linux_asm.S.o 
/b/f/w/src/git/out/stage2-install/bin/clang --sysroot=/b/f/w/src/git/out/sysroots/platform/arm  -I/b/f/w/src/git/out/lib/libomp-arm-static/runtime/src -I/b/f/w/src/git/out/llvm-project/openmp/runtime/src -I/b/f/w/src/git/out/llvm-project/openmp/runtime/src/i18n -I/b/f/w/src/git/out/llvm-project/openmp/runtime/src/include -I/b/f/w/src/git/out/llvm-project/openmp/runtime/src/thirdparty/ittnotify -I/b/f/w/src/git/out/llvm-project/openmp/runtime/src/android -ffile-prefix-map=/b/f/w/src/git/= --target=armv7a-linux-androideabi30 -ffunction-sections -fdata-sections -march=armv7-a -D_LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT=1 --sysroot=/b/f/w/src/git/out/sysroots/platform/arm -O3 -DNDEBUG -fPIC   -D _GNU_SOURCE -D _REENTRANT -D LIBOMP_HAVE_PTHREAD_SETNAME_NP -U_GLIBCXX_ASSERTIONS -MD -MT runtime/src/CMakeFiles/omp.dir/z_Linux_asm.S.o -MF runtime/src/CMakeFiles/omp.dir/z_Linux_asm.S.o.d -o runtime/src/CMakeFiles/omp.dir/z_Linux_asm.S.o -c /b/f/w/src/git/out/llvm-project/openmp/runtime/src/z_Linux_asm.S
/[b/f/w/src/git/out/llvm-project/openmp/runtime/src/z_Linux_asm.S:2485](https://cs.corp.google.com/piper///depot/google3/b/f/w/src/git/out/llvm-project/openmp/runtime/src/z_Linux_asm.S?l=2485):39: error: expected STT_<TYPE_IN_UPPER_CASE>, '#<type>', '%<type>' or "<type>"
    .type __kmp_unnamed_critical_addr,@object
                                                                   ^

I want to check if this error is a mistake from our build scripts or the change.

@rorth
Copy link
Collaborator Author

rorth commented May 7, 2025

It's my fault, unfortunately: I'd forgotten that while the @object form works on the vast majority of ELF targets, ARM/AArch64 is the odd man out. This can be seen in the .section .note.GNU-stack definition a few lines below. I've found
that on all targets currently supported by z_Linux_asm.S, one can assemble %object instead. I'll post a (very lightly) tested patch shortly.

@mstorsjo
Copy link
Member

mstorsjo commented May 7, 2025

It's my fault, unfortunately: I'd forgotten that while the @object form works on the vast majority of ELF targets, ARM/AArch64 is the odd man out.

I think this is only the case for ARM, not AArch64. The reason is that @ is treated as default assembler comment character there - but this isn't the case for AArch64.

rorth added a commit to rorth/llvm-project that referenced this pull request May 7, 2025
PR llvm#138517 broke the Android LLVM builders: ARM doesn't understand the
`@object` form.  As it turns out, one can use `%object` instead, which does
assemble on all targets currently supported by `z_Linux_asm.S`.

Tested by rebuilding `libomp.so` on `sparcv9-sun-solaris2.11`.
@rorth
Copy link
Collaborator Author

rorth commented May 7, 2025

Seems so, still %object assembles on both.

GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
`libomp.so` currently fails to link on SPARC, both Solaris/sparcv9 and
Linux/sparc64:

```
Undefined                       first referenced
 symbol                             in file
__kmp_unnamed_critical_addr         projects/openmp/runtime/src/CMakeFiles/omp.dir/kmp_gsupport.cpp.o
ld: fatal: symbol referencing errors
```

This patch provides the necessary definition. While at it, I noticed
that on non-x86 targets the symbol wasn't marked as `@object`, which
this patch corrects, too.

Tested on `sparcv9-sun-solaris2.11`, `sparc64-unknown-linux-gnu`,
`amd64-pc-solaris2.11`, and `x86_64-pc-linux-gnu`.
rorth added a commit that referenced this pull request May 7, 2025
PR #138517 broke the Android LLVM builders: ARM doesn't understand the
`@object` form. As it turns out, one can use `%object` instead, which
does assemble on all targets currently supported by `z_Linux_asm.S`.

Tested by rebuilding `libomp.so` on `sparcv9-sun-solaris2.11`.
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.

5 participants