[PATCH] D153260: [RISCV][MC] Implement mapping symbols
Job Noorman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 11 01:43:25 PDT 2023
jobnoorman added a comment.
In D153260#4484423 <https://reviews.llvm.org/D153260#4484423>, @kito-cheng wrote:
> This patch LGTM, however D137417 <https://reviews.llvm.org/D137417> has implement more complete mapping symbol support, I am OK to accept/land this part first and then let D137417 <https://reviews.llvm.org/D137417> to implement the rest part, and actually I am prefer this way since this patch implement some detail more right on creating mapping symbol, but I would like defer the final decision to @asb.
Which details are you referring to here? Both patches look mostly logically equivalent to me (ignoring the support for arch strings that this patch doesn't have).
I don't have a strong opinion about which patch should be accepted. I agree that an incremental approach is preferable so it could make sense to merge this one first. However, I don't want to make it more difficult for @LiDongjin to have to rebase their patch on mine.
In D153260#4486289 <https://reviews.llvm.org/D153260#4486289>, @jrtc27 wrote:
> Can LLVM and GNU as both assemble code that has multiple `$d` and or `$x` labels? If so, go with that. If not, there's not much to lose going with the AArch64-style version that uniquifies the labels.
I just noticed that LLVM MC doesn't accept multiple labels with the same name so that isn't an option. This means we'll be incompatible with binutils for now. But it feels like this is something that binutils should fix since the `$x.i` format is compliant with the spec.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153260/new/
https://reviews.llvm.org/D153260
More information about the llvm-commits
mailing list