[PATCH] D153223: [RL78 patch 1] Add RL78 ELF defintions

Sun Jun 18 23:54:48 PDT 2023

jhenderson added inline comments.

================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:919
+  EF_RL78_FU_EXIST = 0x01,
+  // Extension instructions#.
+  EF_RL78_EI_EXIST = 0x08,
----------------
Typo?

================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:921
+  EF_RL78_EI_EXIST = 0x08,
+  // MAA mode
+  EF_RL78_MAA_0 = 0x40,
----------------

================
Comment at: llvm/test/tools/llvm-readobj/ELF/rl78-header-flags.test:4
+# RUN: yaml2obj %s -o %t.fu_exist -DFLAG=FU_EXIST
+# RUN: llvm-readobj -S --file-headers %t.fu_exist | FileCheck --check-prefixes=ALL,FU_EXIST %s
+
----------------
It seems to me like we should have at least a token llvm-readelf/--elf-output-style=GNU test case, as the two code paths start off separately. We need to show that they both recognise the new flags. It may not be necessary to check every single case though.

Up to you, but you could somewhat simplify this test by simply have yaml2obj create the input object with all the flags set at once. You then only need one yaml2obj and one llvm-readobj lines.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153223