[PATCH] D136344: [ELF][RISCV] Merge `riscv.attributes` sections from all input files

Kito Cheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 31 06:36:19 PDT 2022


kito-cheng added inline comments.


================
Comment at: lld/ELF/SyntheticSections.cpp:3758
+                 resultExtensions[extension.first].MinorVersion ==
+                     extension.second.MinorVersion);
+          continue;
----------------
I think this assertion should be removed or use error instead, but I realized one fact soon is: we'll check the version during RISCVISAInfo::parseArchString and it will error out if the version is unsupported, so the assertion should always true for this moment.

Anyway I am OK with that for now, but I would prefer that let @MaskRay to make the final call for that.

Could you add a `TODO:` to the comment, like `TODO: Versions should be the same otherwise parser...`


================
Comment at: lld/ELF/SyntheticSections.cpp:3781
+            *std::max_element(attributes[RISCVAttrs::STACK_ALIGN].begin(),
+                              attributes[RISCVAttrs::STACK_ALIGN].end());
+      }
----------------
This should just error if the stack alignment is different, see psABI for that https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tag_riscv_stack_align-4-uleb128value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136344/new/

https://reviews.llvm.org/D136344



More information about the llvm-commits mailing list