This commit reverts https://reviews.llvm.org/D75039. Concensus appears to
be in favour of assembly-time resolution of these ADR and LDR relocations,
in line with GNU.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
That is my understanding too from my comment in https://reviews.llvm.org/D72892#1923502
I have removed the tests from LLD that used the relocations coming from these instructions and have replaced them with .inst and .reloc. Will be worth making sure that ninja check-lld passes with this change applies. I'm happy for this to be applied. Will need to check with the other reviewers to make sure they are happy.
Interestingly I received some feedback from my earlier GNU as bug report yesterday (https://sourceware.org/bugzilla/show_bug.cgi?id=25406). GNU as will not emit relocations.
I still want to check whether they think rejecting non-STB_LOCAL labels in a potentially preemptible context is acceptable.
I remember that we've seen such reports twice. One is some FreeBSD ARM folks (but at least another two FreeBSD folks told me I can persist). One is a Linux kernel assembly which may have been fixed now. I should add that allowing some Linux kernel in LLVM 9/10 while rejecting some in LLVM 10/11 is ok. There are still a few assembly files not assembled with MC. So being more rigid than GNU as may not cause churn/perceivable problems to Linux kernel builders.
For the patch itself, I am now ok with either way. But please make use of this opportunity to clean up/enhance tests, rather than perform a simple revert.
[ARM][MD] Revert relocation for some pc-relative fixups.
Did you mean MC? Also please mention Thumb. It greatly improves the information conveyed by the commit subject with just 6 more characters.
This broke check-lld everywhere, e.g. http://lab.llvm.org:8011/builders/lld-x86_64-win/builds/71/steps/test-check-all/logs/FAIL%3A%20lld%3A%3Aarm-thumb-undefined-weak.s or http://45.33.8.238/linux/15676/step_11.txt
Please take a look, and revert while you investigate if it's not an easy fix.
If you can give me a an hour or two I can fix, am away from computer at moment. The LLD test will need the adr and ldr instructions replaced with .inst and .reloc I thought I'd done all of these in arm-thumb-adr.s and arm-thumb-ldrlit.s but it looks like I missed this one.
Committed fix to LLD tests under 61bccda9d9d920c72f49025f11e8601daeb096ec and checked it worked with this patch.
@MarkMurrayARM For MC changes, you need ninja check-llvm check-lld. From my experience most affected tests are under llvm/test/{CodeGen,MC,DebugInfo} lld/test.
clang-format not found in user's PATH; not linting file.