[PATCH] D64827: [Xtensa 2/10] Add Xtensa ELF definitions.

Thu Nov 25 00:30:33 PST 2021

jhenderson added a comment.

Still looks good to me, but someone with Xtensa knowledge needs to approve to confirm the flags etc are correct.

================
Comment at: llvm/test/Object/obj2yaml.test:550

-# RUN: yaml2obj %s -o %t-x86-64
+# RUN: yaml2obj --docnum=1 %s -o %t-x86-64
 # RUN: obj2yaml %t-x86-64 | FileCheck %s --check-prefix ELF-X86-64
----------------
jrtc27 wrote:
> jhenderson wrote:
> > jrtc27 wrote:
> > > All other architectures put their input in a separate file to avoid this.
> > Are you referring to the yaml2obj line or the use of `--docnum`? All other files use pre-canned binaries, which are not ideal, as they are fixed and hard to inspect/understand/update. Using yaml2obj is generally preferable for creating objects at test time.
> Oh you're right. Hm. `--docnum=N` for each architecture would start to get a bit silly though... I do wonder why we don't have one .test file for each architecture being tested, then you don't need to play --docnum or Inputs/foo.yaml games (and even for pre-canned binaries it keeps the tests nicely separated rather than having an 800-odd line file).
That would be reasonable. I think it's a legacy of how this test was written. In fact, having an obj2yaml test inside the Object library tests is a bit weird - it should either be in ObjectYAML or more likely tools\obj2yaml. Feel free to tidy things up!

(Apologies, I wrote this commenet ages ago, but forgot to submit it).

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

https://reviews.llvm.org/D64827