[llvm-dev] PSA: debuginfo-tests workflow changing slightly

Sat Nov 4 15:49:36 PDT 2017

If we want to test clang-cl's CodeView output, we don't need
debuginfo-tests in the first place.  We can just use llvm-readobj or
something to dump the codeview from the object files.  If we want to test
that it works with a *real* debugger though, then we need to be testing
PDBs.

One option is to just have clang-cl emit object files and use Microsoft's
linker to link them and produce a PDB.  We will indeed probably do this, as
there is some value in this.  But at the end of the day, that does nothing
to verify that the PDBs *we* generate are kosher.  And the PDBs that we
generate come from LLD.  Actually, testing our PDBs is actually the most
important testing scenario (as opposed to testing our CodeView using MS's
linker to create the PDB), because CodeView is pretty well understood.  I
have high confidence that if our llvm-readobj CodeView tests against
clang-cl generated object files pass, that the PDB generated by Microsoft's
linker will be just fine.  But PDB is still very opaque, and there's a ton
of stuff that goes in there that is *not* CodeView and which is entirely
synthesized by the linker.  The only way to test that is to have LLD
generate the PDB.

On Sat, Nov 4, 2017 at 3:28 PM David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Nov 3, 2017 at 9:34 PM Zachary Turner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> By adding in a different suite, you mean the lld part?  I mean
>> theoretically, but that would be pretty awkward, because the idea behind
>> the lld requirement is that we want to make debuginfo-tests work with
>> clang-cl and CodeView debug info, and for this lld is a hard requirement.
>>
>
> Out of curiosity, why is lld a requirement for this? The system linker is
> sufficient for DWARF testing and I'd have thought the Windows system linker
> would be sufficient for testing clang-cl with CodeView output.
>
> I assume lld is a requirement if you want to test lld's ability to link
> CodeView, but not a requirement for testing clang-cl's CodeView output?
>
>
>>   In that sense, there really isn't a meaningful way to have debug info
>> tests on Windows without lld.  So by putting it another repo, we'd have a
>> "windows debug info tests" repo and a "non windows debug info tests" repo.
>>
>> Although the format of the tests will probably look a little different,
>> and the debuggers being run to verify the tests will definitely be
>> different, conceptually they're really the same thing.
>>
>> Even ignoring the LLD aspect, I think this layout just makes more sense,
>> and when I spoke to several people about it at the dev meeting, I think
>> pretty much everyone was in agreement.  Case in point: test-suite is
>> conceptually very similar to debuginfo-tests, so it's awkward when they use
>> completely different layouts in the source tree and different methods of
>> running the suite.
>>
>> So I actually think this organization is more idiomatic with the way LLVM
>> normally does things, independently of the desire to depend on LLD.
>>
>> In fact, when I started looking into test-suite I got to thinking that
>> maybe debuginfo-tests should actually *be* part of test-suite.  To be
>> clear: I personally have no intention of doing this now or in the future,
>> but the point is that they are similar enough that we should really treat
>> them the same.
>>
>> I admit I'm not familiar with the CI side of things, but from the
>> perspective of someone doing this locally, the transition procedure is:
>>
>> 1) Clone the same git repo as before, but into a different location on
>> disk.
>>
>> 2) Add an additional check step that runs `ninja check-debuginfo`
>>
>> Nothing else should need to change.  I trust you when you say this is a
>> huge undertaking since you know this stuff better than me, but I want to
>> understand where the extra effort comes from.
>>
>> I will need to check what happens if you just do nothing and leave bots
>> running the way they are today.  I guess the way to check this is to clone
>> the repo in the "old" location, apply my patch to that location, and then
>> run ninja check-clang.  It may continue to work, but I haven't tested it.
>> Note that it's the weekend so I can't check this until Monday though.
>>
>> On Fri, Nov 3, 2017 at 9:18 PM Chris Matthews <chris.matthews at apple.com>
>> wrote:
>>
>>> From the CI side moving this stuff around is a huge undertaking.  We
>>> include this repo in every build, they will all need to be fixed and
>>> verified.  It is a lot of work on our side.  Is there a plan for both
>>> system to work side-by-side as we migrate jobs?  Talking to Mike today, we
>>> estimated a week of work to migrate and verify, plus residual failures for
>>> the next month.
>>>
>>> Regarding your motivation for this change, could that test be added in a
>>> different suite?
>>>
>>> I propose we drop these tests from all but one of our OSX bots.  I don’t
>>> see them fail often, and they have a large maintenance burden.
>>>
>>>
>>> On Nov 3, 2017, at 6:12 PM, Vedant Kumar <vsk at apple.com> wrote:
>>>
>>>
>>> On Nov 3, 2017, at 6:09 PM, Zachary Turner <zturner at google.com> wrote:
>>>
>>> llvm-profdata is part of llvm though. It’s perfectly fine for something
>>> in clang to depend on something in llvm. However, clang and lld are two
>>> independent llvm subprojects, neither of which can depend on each other.
>>>
>>> Generally speaking, from a layering perspective, if A depends on B and
>>> C, but B and C are independent, that should be reflected in the structure.
>>>
>>> For example, in CMake we will need to find out if lld is being built,
>>> since it is optional. We would not be able to do this from inside of the
>>> clang tree, without requiring the parent cmake (e.g. llvm) to make sure
>>> that we traversed into lld’s cmake first. This is a clear layering
>>> violation though. Instead, the proper way to do it is have llvm include
>>> both, and the run the debuginfo-tests cmake configuration
>>>
>>>
>>> Got it, thanks.
>>>
>>> vedant
>>>
>>> On Fri, Nov 3, 2017 at 6:00 PM Vedant Kumar <vsk at apple.com> wrote:
>>>
>>>> On Nov 3, 2017, at 3:21 PM, Zachary Turner via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>> Greetings,
>>>>
>>>> If you dont' care about running debuginfo-tests, and don't maintain a
>>>> bot that runs debuginfo-tests, you can stop reading.
>>>>
>>>> I've uploaded a patch [https://reviews.llvm.org/D39605] that changes
>>>> the way you run debuginfo-tests.
>>>>
>>>> Prior to this patch, the way to run them is to clone an external git
>>>> repository into clang/test and then debuginfo-tests will happen
>>>> transparently when you run "ninja check-clang".
>>>>
>>>> After this patch, there will be two workflows depending on if you use
>>>> multi-repo or mono-repo.
>>>>
>>>> multi-repo: You will need to clone debuginfo-tests into llvm/projects,
>>>> then run "ninja check-debuginfo"
>>>>
>>>> mono-repo: pass -DLLVM_ENABLE_PROJECTS="debuginfo-tests", then run
>>>> "ninja check-debuginfo"
>>>>
>>>> The motivation for this change is that planned additions to
>>>> debuginfo-tests require us to be able to make use of lld, and as a result
>>>> the tests need to live somewhere that can access both clang and lld, not
>>>> just clang.
>>>>
>>>>
>>>> I'm not at all opposed to this effort, but I do wonder why this is part
>>>> of the motivation. Tests in clang/test should be able to use any binary in
>>>> <build-dir>/bin, right? E.g we use <build-dir>/bin/llvm-profdata for the
>>>> tests in clang/test/Profile.
>>>>
>>>>
>>>> Furthermore, giving them their own target "check-debuginfo" as opposed
>>>> to being transparently added to check-clang makes more sense from a
>>>> usability perspective.  Finally, this new approach is mono-repo friendly
>>>> whereas the previous one was not.
>>>>
>>>>
>>>> Yep.
>>>>
>>>>
>>>> I'm hoping this won't be too disturbing of a change, but please leave
>>>> comments and issues on this thread or on the code rview.
>>>>
>>>>
>>>> We have several bots which clone debuginfo-tests to tools/clang/test,
>>>> but it shouldn't be too much of a hassle to migrate them. I've CC'd Mike
>>>> and Chris as a heads-up (or in case they have anything to add :).
>>>>
>>>> thanks,
>>>> vedant
>>>>
>>>>
>>>>
>>>> Thanks!
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171104/68fbaabc/attachment.html>