[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