Skip to content

[LLD][PowerPC] Fix "unrecognized instruction for IE to LE R_PPC64_TLS" for PowerPC targets #64424

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.

Sign up for GitHub

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

Jump to bottom
Closed
amy-kwan opened this issue Aug 4, 2023 · 13 comments · Fixed by llvm/llvm-project-release-prs#675
Closed

[LLD][PowerPC] Fix "unrecognized instruction for IE to LE R_PPC64_TLS" for PowerPC targets #64424

amy-kwan opened this issue Aug 4, 2023 · 13 comments · Fixed by llvm/llvm-project-release-prs#675
Assignees
@amy-kwan
Labels
backend:PowerPC lld:ELF release:backport release:merged
Milestone
LLVM 17.0.X Release

Comments

@amy-kwan
Copy link
Contributor

amy-kwan commented Aug 4, 2023 •
edited
Loading

As a result of https://reviews.llvm.org/D153645, there is a failure when linking an msan libcxx test during check-runtimes:

$ cd /home/amyk/lldtest-mainbranch/runtimes/runtimes-bins/compiler-rt/lib/msan/tests && /home/amyk/lldtest-mainbranch/./bin/clang++ MSAN_INST_TEST_OBJECTS.msan_test.cpp.powerpc64le-with-call.o MSAN_INST_TEST_OBJECTS.msan_test_main.cpp.powerpc64le-with-call.o MSAN_INST_GTEST.gtest-all.cc.powerpc64le-with-call.o /home/amyk/lldtest-mainbranch/runtimes/runtimes-bins/compiler-rt/lib/msan/tests/../libcxx_msan_powerpc64le/lib//libc++.a /home/amyk/lldtest-mainbranch/runtimes/runtimes-bins/compiler-rt/lib/msan/tests/../libcxx_msan_powerpc64le/lib//libc++abi.a -o /home/amyk/lldtest-mainbranch/runtimes/runtimes-bins/compiler-rt/lib/msan/tests/./Msan-powerpc64le-with-call-Test -Wl,--color-diagnostics -nostdlib++ -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -fsanitize=memory -ldl -m64 -fno-function-sections -m64 -fno-function-sections
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
ld.lld: error: unrecognized instruction for IE to LE R_PPC64_TLS
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

This issue occurs on both LLVM 17.0.0 rc1 and main.
A patch to fix this issue will follow.
@amy-kwan amy-kwan added this to the LLVM 17.0.X Release milestone Aug 4, 2023
@amy-kwan amy-kwan self-assigned this Aug 4, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Aug 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Aug 4, 2023

@llvm/issue-subscribers-backend-powerpc

@amy-kwan
Copy link
Contributor Author

amy-kwan commented Aug 4, 2023

Currently working on a fix, so this issue will be assigned to me.

@amy-kwan amy-kwan changed the title [LLD][PowerPC] Fix "unrecognized instruction for IE to LE R_PPC64_TLS" for LLVM 17.0.0 rc1 [LLD][PowerPC] Fix "unrecognized instruction for IE to LE R_PPC64_TLS" for PowerPC targets Aug 4, 2023
@nemanjai
Copy link
Member

nemanjai commented Aug 5, 2023

Is this because new instructions were added for TLS loads/stores?

@amy-kwan
Copy link
Contributor Author

amy-kwan commented Aug 5, 2023

Is this because new instructions were added for TLS loads/stores?

Yeah, the issue occurs because I am introducing more X-Form loads/stores that I can produce tryTLSXFormLoad/tryTLSXFormStore in PPCISelDAGToDAG.cpp in https://reviews.llvm.org/rG11b71ade51e0d1f90f1c68a7552a11f7e85eace1, and they are not accounted for in LLD.

I believe they need to be handled in LLD because relaxTlsIeToLe() will try to relax initial-exec to local-exec sequences if possible, and the types of load/stores that are handled are specifically implemented inside lld/ELF/Arch/PPC64.cpp.

I did not catch this before as I realized I did not build libc++, which is where the test is coming from.

@nemanjai
Copy link
Member

nemanjai commented Aug 6, 2023

If the fix is going to take a while and this is blocking the libc++ build, perhaps it is better to temporarily pull the patch until a complete fix is available.

@nikic nikic moved this from Needs Triage to Needs Fix in LLVM Release Status Aug 15, 2023
@MaskRay
Copy link
Member

MaskRay commented Aug 18, 2023

If the fix is going to take a while and this is blocking the libc++ build, perhaps it is better to temporarily pull the patch until a complete fix is available.

Removing https://reviews.llvm.org/D153645 for just https://github.com/llvm/llvm-project/tree/release/17.x LGTM, to not cause compatibility issues with lld TLS optimization.

@tru
Copy link
Collaborator

tru commented Aug 21, 2023

Are we still targetting a fix for 17.X?

@amy-kwan
Copy link
Contributor Author

I forgot to link the patch back here, but I have https://reviews.llvm.org/D158197 up for review.

@EugeneZelenko EugeneZelenko added lld:ELF and removed lld labels Aug 22, 2023
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2023

@llvm/issue-subscribers-lld-elf

@amy-kwan
Copy link
Contributor Author

Are we still targetting a fix for 17.X?

@tru Yes, to answer the question explicitly, I believe we are still targeting a fix for 17.x with the patch I have up: https://reviews.llvm.org/D158197

@amy-kwan
Copy link
Contributor Author

Reopening issue to cherry-pick the fix into release/17.x:
/cherry-pick 698b45a

@amy-kwan amy-kwan reopened this Aug 31, 2023
@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2023

/branch llvm/llvm-project-release-prs/issue64424

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 31, 2023
@amy-kwan @llvmbot
[PowerPC][lld] Account for additional X-Forms -> D-Form/DS-Forms load…
2cdd381 
…/stores when relaxing initial-exec to local-exec

D153645 added additional X-Form load/stores that can be generated for TLS accesses.
However, these added instructions have not been accounted for in lld. As a result,
lld does not know how to handle them and cannot relax initial-exec to local-exec
when the initial-exec sequence contains these additional load/stores.

This patch aims to resolve llvm/llvm-project#64424.

Differential Revision: https://reviews.llvm.org/D158197

(cherry picked from commit 698b45aa902de4d30c798e8d6bd080c8e31bade8)
@llvmbot llvmbot mentioned this issue Aug 31, 2023
Merged
@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2023

/pull-request llvm/llvm-project-release-prs#675

@tru tru moved this from Needs Fix to Needs Review in LLVM Release Status Sep 1, 2023
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 1, 2023
@amy-kwan @tru
[PowerPC][lld] Account for additional X-Forms -> D-Form/DS-Forms load…
2b3336f 
…/stores when relaxing initial-exec to local-exec

D153645 added additional X-Form load/stores that can be generated for TLS accesses.
However, these added instructions have not been accounted for in lld. As a result,
lld does not know how to handle them and cannot relax initial-exec to local-exec
when the initial-exec sequence contains these additional load/stores.

This patch aims to resolve llvm/llvm-project#64424.

Differential Revision: https://reviews.llvm.org/D158197

(cherry picked from commit 698b45aa902de4d30c798e8d6bd080c8e31bade8)
@tru tru moved this from Needs Review to Done in LLVM Release Status Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@amy-kwan amy-kwan

Labels
backend:PowerPC lld:ELF release:backport release:merged
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#64424 llvm/llvm-project-release-prs
6 participants
@tru @MaskRay @EugeneZelenko @nemanjai @amy-kwan @llvmbot