[PATCH] D79545: [VE] Implements minimum MC layer for VE (3/4)

[Kazushi Marukawa via Phabricator via llvm-commits]

kaz7 marked 4 inline comments as done and an inline comment as not done.
kaz7 added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:1222-1223
 
+  case ELF::EM_VE:
+    return Triple::ve;
+
----------------
kaz7 wrote:
> jhenderson wrote:
> > Test coverage?
> Is there any idea how to test it?  I've checked other tests and found llvm-ifs which I've never used.  Using llvm-ifs is correct way to implement test for this?  Thanks.
Not sure about llvm-ifs.  Please advise me.  Thanks.


================
Comment at: llvm/lib/Object/ELF.cpp:148-154
+  case ELF::EM_VE:
+    switch (Type) {
+#include "llvm/BinaryFormat/ELFRelocs/VE.def"
+    default:
+      break;
+    }
+    break;
----------------
kaz7 wrote:
> jhenderson wrote:
> > Please add an llvm-readobj test "reloc-types-elf-ve.test" to test/tools/llvm-readobj/ELF similar to the existing "reloc-types-elf-*.test" files for other machines.
> Thank you for examples.  I'll add a test.  Having unknown errors, ATM.
Add a test.


================
Comment at: llvm/lib/Object/ELF.cpp:201-202
     break;
+  case ELF::EM_VE:
+    return ELF::R_VE_RELATIVE;
   default:
----------------
kaz7 wrote:
> jhenderson wrote:
> > Test coverage?
> I will try it in above "reloc-types-elf-*.test".
I'm going to remove this modifications since VE is not using SHT_RELR atm.  Thank you very much for giving me opportunity to correct this. 


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:666-668
+  case ELF::EM_VE:
+#include "llvm/BinaryFormat/ELFRelocs/VE.def"
+    break;
----------------
kaz7 wrote:
> jhenderson wrote:
> > There should be some yaml2obj/obj2yaml testing to show that VE relocation enums are supported in those tools.
> Thanks.  I'll try implement one.
Add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79545