This patch adds necessary definitions for LoongArch ELF files, including
relocation types. Also adds initial support to ELFYaml, llvm-objdump,
and llvm-readobj in order to work with LoongArch ELFs.
Depends on D115857
Differential D115859
[LoongArch 2/6] Add ELF machine flag and relocs for upcoming LoongArch target SixWeining on Dec 16 2021, 12:48 AM. Authored by
Details This patch adds necessary definitions for LoongArch ELF files, including Depends on D115857
Diff Detail
Unit Tests Event TimelineComment Actions Two nits. Otherwise looks good. Your comment references llvm-objdump, but there's nothing testing llvm-objdump here. I imagine it's just a case of "it just works", due to the Object library changes? I haven't checked that the LOONGARCH definitions match up with any official spec. Do you have anything you could cite for these values i.e. ELF generic-abi mailing list for the EM value, and some psABI doc for the relocations? Not that I think this particular change needs it, but has there been any particular buy-in from the community for integrating this architecture at all? (In this case, I don't think it's necessary, but for wider changes, it probably is). Please note that I won't be online until the 4th of January, after today's work, so may be slow to respond to anything.
Comment Actions
Comment Actions Thanks @jhenderson and I have updated this revision to address your comments. Comment Actions This looks good to me too. I will wait for more people to have a look at it before approving, especially during the long break until January.
Comment Actions Thanks for your comments @xen0n and I will update the revision to address them.
Comment Actions Hi it seems like this patch caused an error in the lldb build machines: /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/BinaryFormat/ELF.h:877:1: error: expected identifier ^ /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/BinaryFormat/ELF.h:876:7: error: expected ';' after enum ^ ; /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/BinaryFormat/ELF.h:876:1: warning: declaration does not declare anything [-Wmissing-declarations] link to bot here: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/41227/ Please fix or revert your patch as soon as possible! Comment Actions I'm not sure how that's possible, given that that the code here isn't doing anything different than any other backend. The only thing I could think about was the extra newline on LoogArch's enum defs that wasn't present in any other arch. So I committed 695b629edd03 removing that newline. It's a wild guess, it's not relevant and it shouldn't hurt, but if that's what it was (because macros are stupid), then let me know. If not, then we need to understand how you're building it because the other bots are fine and that is a build failure. Ex. https://lab.llvm.org/buildbot/#/builders/188/builds/9576 Comment Actions It did not work. Can you explain how you build and why is this only breaking on your bot? Comment Actions If I'm making a wild guess, it doesn't look like LoongArch.def is mentioned as a textual header in module.modulemap. Comment Actions So, I can reproduce the error now, but I'm not sure which one of the many modulemap file you're referring to. I can't find any other ELFRelocs file in any of the many modulemap files in any of the monorepo projects. Comment Actions Thanks for fixing the build error. I just submit a new patch D119519 to sort the textual headers by alphabetical order. |
I imagine that this is copied from other files, but this blank line can be deleted.
I'm not sure why we don't include a license header at the top here, but I presume that's the case in the other files too?