[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

Tue Nov 13 15:47:24 PST 2018

I am not sure if that’s the right solution for a couple of reasons:

  1.  As far as I can tell only clang calls use_clang (and only lld calls use_lld), while the other projects such as lld and llvm rely on the environment to be setup correctly
  2.  Lld also has tests that invoke clang-cl and they pass – while the ones in LLDB do not, so the invocation of use_clang is not necessary for the tests to pass (maybe?)
  3.  LLDB allows us to specify whether to use gcc or clang as well as the path and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so we should first decide what scenarios we want to support before trying to make this work and possibly making it even more confusing and complicated

Do you know what the answer for 3) is? What compilers are valid to specify for the lit/suite/unittests via the various parameters?

Thanks,
-Stella

From: Zachary Turner <zturner at google.com>
Sent: Tuesday, November 13, 2018 3:27 PM
To: Stella Stamenova <stilis at microsoft.com>
Cc: reviews+D54009+public+0e164460da8f1d7f at reviews.llvm.org; pavel at labath.sk; chris.bieneman at me.com; dccitaliano at gmail.com; aleksandr.urakov at jetbrains.com; jdevlieghere at apple.com; abidh.haq at gmail.com; teemperor at gmail.com; ki.stfu at gmail.com; mgorny at gentoo.org; dan at su-root.co.uk; jfbastien at apple.com; lldb-commits at lists.llvm.org; llvm at inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I believe that is correct, and perhaps part of the problem.  In other projects we call llvm_config.use_clang(), and that actually explicitly creates a substitution so that if someone writes "clang" they'll get an error that says "use %clang instead".  Because we have the exact path, that isn't happening here.  Perhaps the proper fix is to add a keyword argument to llvm_config.use_clang() so that it can be called as llvm_config.use_clang(path=p).  This way we can get the proper substitutions created and we would find out about this.

I can play around with this some, but does it sound reasonable to you?  I'm not sure how to hit all the edge cases because everything is already working for me (for some reason), so it won't be obvious if I fix the problem for those people for whom it's broken.

On Tue, Nov 13, 2018 at 3:16 PM Stella Stamenova <stilis at microsoft.com<mailto:stilis at microsoft.com>> wrote:
I took a brief look and I have a question about the usage of clang (rather than clang-cl).

In general I would agree that we have an exact path of clang (or gcc) that we are trying to use and they’re specified by using %cc and %cxx in the test files, but there are a number of test files that simply use clang e.g.:

SymbolFile\DWARF\find-variable-dwo.cpp:3:// RUN: clang %s -g -gsplit-dwarf -c -emit-llvm -o - --target=x86_64-pc-linux -DONE

In this case, are we not going to pick up whatever clang happens to be in the path instead of one that was explicitly specified? Is this intentional?

Thanks,
-Stella

From: Zachary Turner <zturner at google.com<mailto:zturner at google.com>>
Sent: Tuesday, November 13, 2018 2:46 PM
To: reviews+D54009+public+0e164460da8f1d7f at reviews.llvm.org<mailto:reviews%2BD54009%2Bpublic%2B0e164460da8f1d7f at reviews.llvm.org>
Cc: Stella Stamenova <stilis at microsoft.com<mailto:stilis at microsoft.com>>; pavel at labath.sk<mailto:pavel at labath.sk>; chris.bieneman at me.com<mailto:chris.bieneman at me.com>; dccitaliano at gmail.com<mailto:dccitaliano at gmail.com>; aleksandr.urakov at jetbrains.com<mailto:aleksandr.urakov at jetbrains.com>; jdevlieghere at apple.com<mailto:jdevlieghere at apple.com>; abidh.haq at gmail.com<mailto:abidh.haq at gmail.com>; teemperor at gmail.com<mailto:teemperor at gmail.com>; ki.stfu at gmail.com<mailto:ki.stfu at gmail.com>; mgorny at gentoo.org<mailto:mgorny at gentoo.org>; dan at su-root.co.uk<mailto:dan at su-root.co.uk>; jfbastien at apple.com<mailto:jfbastien at apple.com>; lldb-commits at lists.llvm.org<mailto:lldb-commits at lists.llvm.org>; llvm at inglorion.net<mailto:llvm at inglorion.net>
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I think it must be related to setting up the environment in which to run clang.  In all other projects we call llvm_config.use_clang() which is in llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path of a clang we are trying to use, we skip this function in LLDB's lit configuration files.  But there is also a lot of other logic in that function, so perhaps it's some of that logic that's necessary.

On Mon, Nov 12, 2018 at 9:02 AM Aleksandr Urakov via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> wrote:
aleksandr.urakov added a comment.

But all compiles without errors if I run this manually:

  clang-cl -m32 /Z7 /c /GS- C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD54009&data=02%7C01%7Cstilis%40microsoft.com%7C128cd278a84f49a36ec708d649bf95ef%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636777484542478768&sdata=f4tnP6dmSOdJ41NsENxeeiLAnYyq4OCGDqs1MOQ6C9k%3D&reserved=0>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181113/6bbf5f76/attachment-0001.html>