[PATCH] D130238: [LoongArch] Parse LoongArch base ABI in ObjectYAML and llvm-readobj

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Where you've added the contents to different files, you seeme to have added them at differenet relative places. For ease of reading, it would be better to keep platform-specific code in the same order in each file (I acknowledge it isn't perfect as it stands anyway, but we should avoid making it worse). Specifically, in same places, you've made your changes before the corresponding AVR block, and in others you've made them after. It don't care about the exact ordering, as long as it's consistent across the files.



================
Comment at: llvm/test/Object/LoongArch/elf-flags.yaml:1
+# RUN: yaml2obj --docnum=1 %s > %t-lp64s
+# RUN: llvm-readobj -h %t-lp64s | FileCheck --check-prefix=READOBJ-LP64S %s
----------------
I don't think the Object library is the place for this testing, as the test tests functionality in ObjectYAML and llvm-readobj code, not in the Object library. I think you should split the test into two tests as follows:

1) In test/ObjectYAML, have a test that uses yaml2obj to generate object files that are converted back to YAML using obj2yaml. Check the resultant YAML contains the appropriate flags.
2) In test/tools/llvm-readobj, use yaml2obj to generate object files, and use llvm-readobj (and llvm-readelf) to inspect them.


================
Comment at: llvm/test/Object/LoongArch/elf-flags.yaml:62-67
+# YAML-LP64S:      FileHeader:
+# YAML-LP64S-NEXT:   Class:           ELFCLASS64
+# YAML-LP64S-NEXT:   Data:            ELFDATA2LSB
+# YAML-LP64S-NEXT:   Type:            ET_EXEC
+# YAML-LP64S-NEXT:   Machine:         EM_LOONGARCH
+# YAML-LP64S-NEXT:   Flags:           [ EF_LOONGARCH_BASE_ABI_LP64S ]
----------------
As you're only testing the flags, you don't need most of these lines. Just the `Flags` line would be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130238