[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

Wed Oct 30 11:12:10 PDT 2019

labath added a comment.

Yes, I understand what you're saying, but I am afraid the situation is not as clear-cut as that. I would be very happy if it was, but it's definitely not the current situation, and given the complexities involved, I'm not sure if we should even aim for that goal. I mean, a lot of times these things end up being down to the personal preference of the author (or reviewer), but there are cases where writing a test of the appropriate category (based on your above classification) would be very hard. Lemme give some examples:

- there is a subclass of "dotest" tests which test lldb's python api (most of them tagged with the "pyapi" category), which test lldb's python api. These definitely don't benefit from being run with multiple compilers (in fact, a lot of them don't run the compiler at all). I don't think it makes sense to change these into "lit" tests.
- another class of "dotest" tests are those which test some interactive behavior of lldb (inferior control, thread plans, stepping and stuff). While these may touch debug info peripherally, it's usually not their main focus, and I don't think we should be relying on them on testing debug info. However, being able to run them in a more "interactive" manner is usually more convenient.
- "pexpect" tests definitely don't/shouldn't test debug info, but they currently exist only in the "dotest" suite (though they are fairly independent, and they could be extracted if we wanted to)
- on the other side of the fence, there are "lit" tests which do not test debug info, or even "debugging", but they still need a host executable, because debugging something is the only way to trigger some functionality. For instance the "register" tests need a functional executable to check that they can read/write registers from it. Unwind tests need a running executable because we can't test even basic unwind without a running process now. Reproducers need a debuggable executable too, but they don't really care about it's contents, etc.

So overall, while there definitely are tests which would be better off moved from one category or the other, I am doubtful that we'll be able to enforce a separation where all "configurable" tests are dotest tests, and all "lit" tests hardcode a triple. The one barrier we have right now is that it is not possible to run the "lit" suite with a different compiler, and that's something I think we should keep. And we can definitely change some %clang_hosts to a hard coded triple (I think that would remove some "UNSUPPORTED: windows" stanzas) and/or be stricter about using %clang_host in the future. In fact I am happy that you spoke out against %clang_host, because when I was  going through these tests, I got the impression that the only tests which have a specific triple hard-coded are those that I have written myself. :)

For the test which don't need a host executable, I don't think that running them for each available target would be useful, as they generally don't care about those details. What they sometimes care about is the object file format (because of dwarf differences in elf vs. macho), so it may make sense to introduce some substitution like  %elf_triple and %macho_triple, which would choose some elf flavour of one of the configured targets. However, I am afraid that even that would be of limited usefulness, as it's pretty hard to make a specific test which is target agnostic (I tried) -- there are differences in pointer sizes, and even the different assemblers can't agree on what they use as a comment character.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69619