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

[Lu Weining via Phabricator via llvm-commits]

SixWeining added inline comments.


================
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
----------------
jhenderson wrote:
> 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.
Thanks. But seems other archs also put the test in the same place. And I saw no platform-specific test in test/ObjectYAML. What should I do?


================
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 ]
----------------
jhenderson wrote:
> As you're only testing the flags, you don't need most of these lines. Just the `Flags` line would be enough.
Sounds good. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130238