[PATCH] D136344: [ELF][RISCV] Merge `riscv.attributes` sections from all input files
Elena Lepilkina via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 31 08:13:06 PDT 2022
eklepilkina added inline comments.
================
Comment at: lld/ELF/SyntheticSections.cpp:3758
+ resultExtensions[extension.first].MinorVersion ==
+ extension.second.MinorVersion);
+ continue;
----------------
kito-cheng wrote:
> 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...`
Yes, it's true I can remove completely.
================
Comment at: lld/ELF/SyntheticSections.cpp:3781
+ *std::max_element(attributes[RISCVAttrs::STACK_ALIGN].begin(),
+ attributes[RISCVAttrs::STACK_ALIGN].end());
+ }
----------------
kito-cheng wrote:
> 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
Thank you for link to documentation with merge rules. Fix merging attributes with these rules.
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